-
Notifications
You must be signed in to change notification settings - Fork 4
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
Routing fixes #66
Routing fixes #66
Conversation
- Remove comments and unused imports and variables - Move styling from global CSS to within individual files with makeStyles - Remove outer scroll from page with search results Resolves #56
- Change 'Get URL' to 'Upload Label' - Move 'Upload Label' button inline to input field - Update 'Nodes' dropdown to 'Node' - Move 'Save' and 'Submit for Review' buttons to be inline - Rearrange placement and handling of the buttons above and their error messsages Resolves #48
-Added react router to simulate pages and allow navigation button use.
- by Keywords text box - by 'publicly available?' for general "Questions about process or information requested? See FAQs or contact the PDS Operator..." Resolves #50
-Added a button that resets the DOI reserve screen after a submission has been completed.
* issue-56: Add additional help information throughout UI Update Release/Update page to design similar to Create Add remaining enhancements per initial implementation of mockups Address requested changes on closed PR
* reserve-new-after-submit-22: Add Reset DOI Reserve Submission Button #22 # Conflicts: # src/components/Create.jsx # src/components/Reserve.jsx # src/reducers/appReducer.js
* routing: Add URL Routing #52 # Conflicts: # src/actions/appAction.js # src/components/Home.jsx # src/components/Release.jsx # src/components/SearchResults.jsx # src/reducers/appReducer.js
-Fixed link to excel sheet file reloading page. -Fixed the FAQ tooltip link. -Fixed link to release page after a reservation is complete. -Fixed reservation url saying [object] instead of doi. -Fixed registered page not showing.
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.
That is a big one, thanks Eddie for putting that together.
Only change I would ask for is the button "Reserve a new DOI" should be "create a new DOI" or "create another DOI" (since we are asking the question again Has the data been registered and made publicly available?
in the next step, the user can reserve or draft/release a DOI).
Otherwise that sounds good to me. I will wait for @c-suh 's review before I merge.
Thanks,
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.
This is looking good, @eddiesarevalo ! I did notice a few things and pushed a change for an overlooked placeholder link.
- Search behavior: IIRC, the latest decision regarding the default view for the Search page is that it's not supposed to display any results until the search button (or enter from the search bar) is entered.
- Search behavior: clicking the 'X' in the search bar to clear the search - should this clear the results as well as the text in the search bar?
- There's a chunk of code in SearchResults.jsx (lines 187-279) which is mostly comments. Either the commented out part can be deleted or those lines can be replaced with
null
.
@tloubrieu-jpl, I left a review above, but you might want to sign off on my points 1 and 2 as "fine as is"? |
Thanks @c-suh , I will create new tickets for your comments so that I can merge now. Thanks |
Hi @eddiesarevalo, Were you able to change the buton label to create a new DOI after one is reserved ? Thanks, Thomas |
-Changed the label of the reset DOI creation form button after a DOI is reserved.
-Changed the basename of the app to '/dois'
…into routing-fixes
@tloubrieu-jpl I changed the label for the button. I also changed the basename subdiretory to '/dois' |
thanks @eddiesarevalo , is the basename subdiretory hardcoded ? for the production deployment we want to deploy the build application in an apache directory and ideally the application would be agnostic of which apache directory it is deployed in. |
-Set the router base name to allow any subdirectory.
@tloubrieu-jpl Try to test this latest commit. It should allow deployment on any sub directory. I have only been able to test locally but you can try it on your server. Use |
Thanks @eddiesarevalo that works. There is only a minor issue that does not prevent from merging and deploying on pds-gamma. I created a ticket #70 |
Fixed the issues caused by merging #22 #52 #57 together. The pull requests had to be updated with the router code.
This also adds #65 through #52
resolves #22
resolves #52
resolves #57
resolves #65
resolves #64
I need @c-suh to verify that the latest changes are there and functional. I tested end to end with a dummy excel doi. @c-suh can use her tests to double check everything is working.