-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add react-based frontend #69
Conversation
Thanks @trungleduc for working on this! Do you have a screenshot around for what the new frontend / UI looks like, even if it's still a draft for now and subject to changes? Thanks! |
@jtpio I updated the first post. |
ddeedca
to
34ebeb6
Compare
7e0fcc0
to
7c201cc
Compare
@jtpio this PR is ready again |
.yarnrc.yml
Outdated
@@ -0,0 +1,2 @@ | |||
nodeLinker: node-modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this simplifies the setup, we could also use npm
instead of yarn
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed, updated!
Co-authored-by: Jeremy Tuloup <[email protected]>
.github/workflows/build.yml
Outdated
uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1 | ||
|
||
- name: Lint the application | ||
shell: bash -l {0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this shell
(and the others below) could be removed if we use the base-setup
action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @trungleduc!
It's also really nice to have UI tests and reference snapshots 👍
Maybe we could even replace the screenshots in the README by the snapshots from the UI test, so the README stays up-to-date when new changes land?
I didn't go through the whole frontend code in the details, but overall it looked good. We can always iterate on it later if something needs to be fixed. |
I will do it in the follow-up PR |
Code changes
Screenshot