React Frontend #205
Merged
React Frontend #205
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- also add bootstrap and react-bootstrap
- update navbar with target values ("_self" vs "_blank")
- also fix font in globals.css - fix semi-colon in _app.tsx - move unused construction image to public/ - make urls display green, with white hover
- to be deleted upon completion of frontend transition
tkkuehn
requested changes
May 25, 2023
There was a problem hiding this comment.
Biggest comment: I'm having trouble getting this to run (with docker-compose up --build).
Generally for this (admittedly wonky) setup, I think we'll want to check in afidsvalidator-react/dist/index.js so I can just clone and get it up and running. (Although we could build in the Dockerfile theoretically...). Anyway after building I get the console error:
Uncaught ReferenceError: currentUser is not defined
oi NavBar.tsx:67
<anonymous> index.ts:4
<anonymous> index.js:46
NavBar.tsx:67:12
I'd also install eslint, eslint-plugin-react, and prettier, and add them to the CI. I'll send you an example .eslintrc to get you going (but feel free to make any changes you'd like).
Once you've taken a stab at this stuff, I'll take another look.
afidsvalidator/static/lib/afidsvalidator-react/src/components/Footer.tsx
Outdated
Show resolved
Hide resolved
afidsvalidator/static/lib/afidsvalidator-react/src/components/Footer.tsx
Outdated
Show resolved
Hide resolved
afidsvalidator/static/lib/afidsvalidator-react/src/components/NavBar.tsx
Outdated
Show resolved
Hide resolved
afidsvalidator/static/lib/afidsvalidator-react/src/components/NavBar.tsx
Outdated
Show resolved
Hide resolved
afidsvalidator/static/lib/afidsvalidator-react/src/components/NavBar.tsx
Outdated
Show resolved
Hide resolved
- still need to pass along currentUser properly
- user auth
- pass current_user.is_authenticated from backend to frontend bool
param for navbar
- linters
- added eslint and typescript plugins
- set up linters
- add scripts for linting and formatting
- performed linting on existing files
tkkuehn
requested changes
Sep 26, 2023
Closed
tkkuehn
approved these changes
Sep 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Change to switch some of the frontend from pure HTML/CSS/JS to React+Typescript. Relatively minor changes here (primarily to the navbar and footer), but will be beneficial in the long run for building out an admin UI. Also fixes some visual discrepancies on the page.
One thing that needs to be checked / tested is the login/logout functionality since this was moved from Jinja (Flask) to React. Will also need to slightly change the deployment.
Types of changes
What types of changes does your code introduce? Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!blackwith the-l 79flag.Notes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.
PR template was adopted from appium