-
Notifications
You must be signed in to change notification settings - Fork 49
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
Migrate to react #351
Migrate to react #351
Conversation
04d8069
to
4cbf691
Compare
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.
Absolutely fantastic work, thanks so much!!
web/.env
Outdated
@@ -0,0 +1,2 @@ | |||
REACT_APP_BACKEND_PORT=5000 |
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.
Is this still needed with the local dev proxy? Also does not match with the docs :)
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.
Yeah, I just checked, that is something that was left over. The way it is documented is correct here, we need a BACKEND_PORT and a BACKEND_HOST variable that are used to configure the proxy.
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.
I removed the whole file, I'd recommend using a .env.development.local
file anyway, as documented.
web/package.json
Outdated
"start": "react-scripts start", | ||
"build": "react-scripts build", | ||
"test": "react-scripts test --watchAll=false --testMatch **/src/**/*.test.ts --passWithNoTests", | ||
"jest": "react-scripts test --watchAll=false --testMatch **/src/**/*.test.ts --passWithNoTests", |
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.
Would remove this as 'test' should be enough i guess?
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.
I think the pipline uses npm jest
that's why
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.
I would suggest to change this in the pipeline and remove the 'jest' command
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.
Sounds reasonable 👍
@reglim are the shadows in the banner now removed, they are still in the screenshots :) |
You can now upload documentation in a new ui with drag and drop :)
I cleaned up some bad decisions :)
* If latest was specified, the version with the latest tag is selected by default * Removed the version from the link on the home page * Add The Upload Button to the Help page * Small Documentation improvements
Data was often fetched more than once, and handling errors was difficult, so I switched to using SWR (a data fetching library). Also fixed some bugs with Error Messages.
4cbf691
to
054bd6e
Compare
Yes, they're gone, I just haven't updated the images yet. |
Minor stuff, like using the arrow sytax everywhere and moving some interface definitions around. Also, added a message provider for unified banners, and implemented a rule to disallow projects with names that conflict with pages on docat web.
054bd6e
to
a571620
Compare
I recreated the whole project in React as Vue 2 is not supported anymore and the transition to Vue 3 proved difficult. While we were at it, I also improved the style a bit by making it more intuitive and modern, especially on the Delete, Claim and Upload Page. I also added the Search Banner and Search Page that were removed from the PR #320
fixes: #324