Skip to content
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

Feature/208 link to userguide #79

Merged
merged 3 commits into from
Apr 26, 2023
Merged

Conversation

cschwinderg
Copy link
Collaborator

Related Issues:

Main Changes:

  • Added a placeholder for the user's guide.

Steps To Test:

  1. Verify the user's guide button at the top of the page works.
  2. Verify the national downloads page still works
  3. Navigate to http://localhost:9090/api/getFile/path/config/services.json
  4. Verify the file is opened in the browser

Copy link
Collaborator

@maxdiebold-erg maxdiebold-erg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, I like the switch to using path parameters in the getFile endpoint, that should help clean up the displayed link on the Swagger page, too.

I noted this in #76, but I think we should switch to an actual navbar (and a responsive one), since we're getting a number of items at the top of the page and will likely add more. Also, I'd say most of the "buttons" should be changed to links, for accessibility reasons, with the exception of the Glossary button. We can discuss this later, though.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@cschwinderg
Copy link
Collaborator Author

I noted this in #76, but I think we should switch to an actual navbar (and a responsive one), since we're getting a number of items at the top of the page and will likely add more. Also, I'd say most of the "buttons" should be changed to links, for accessibility reasons, with the exception of the Glossary button. We can discuss this later, though.

That is a good point. I'm going to go ahead and merge this in as it is now and we can discuss this later. EPA may want to add text to the top, which may make it easier to do links like what we do for LEW.

@cschwinderg cschwinderg merged commit 2485ccc into develop Apr 26, 2023
@cschwinderg cschwinderg deleted the feature/208-link-to-userguide branch April 26, 2023 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants