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

8395 [Manage Public Key] Add new route, and basic page #8665

Merged
merged 8 commits into from Mar 10, 2023

Conversation

penny-lischer
Copy link
Collaborator

@penny-lischer penny-lischer commented Mar 9, 2023

This PR ...

Adds a new route and page for the Public Key Tool

Test Steps:

  1. Navigate to /resources/manage-public-key
  2. Verify that you do not see the new 'Manage Public Key' page
  3. Log into RS as a sender or admin
  4. Navigate to /resources/manage-public-key
  5. Verify that you see the new 'Manage Public Key' page

Changes

  • Added new route /resources/manage-public-key
  • Added new 'Manage Public Key' page

Checklist

Testing

  • Tested locally?
  • Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?
  • (For Changes to /frontend-react/...) Ran npm run lint:write?
  • Added tests?

Linked Issues

@penny-lischer penny-lischer self-assigned this Mar 9, 2023
@penny-lischer penny-lischer added frontend Work Type label to flag work related to the front-end websites experience Team label to flag issues owned by the Experience Team labels Mar 9, 2023
@penny-lischer penny-lischer temporarily deployed to staging March 9, 2023 17:16 — with GitHub Actions Inactive
@penny-lischer penny-lischer temporarily deployed to staging March 9, 2023 17:58 — with GitHub Actions Inactive
@penny-lischer penny-lischer force-pushed the experience/8395/public-key-tool branch from 4c7c911 to 0006c1a Compare March 9, 2023 18:41
@penny-lischer penny-lischer temporarily deployed to staging March 9, 2023 18:41 — with GitHub Actions Inactive
@penny-lischer penny-lischer temporarily deployed to staging March 9, 2023 19:46 — with GitHub Actions Inactive

export function ManagePublicKey() {
return (
<div className="grid-container margin-bottom-5 tablet:margin-top-6">

Choose a reason for hiding this comment

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

Let's use paddings instead of margins when we're restricting widths/heights of contents; margins are meant for spacings between elements rather than dictating the descendant content flow, and vertical margins are also susceptible to collapse, which can happen unintentionally especially between top-level elements of components.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just following the way we have it throughout the site but can change it. Will also change from <div> to <section> per your comment below.


export function ManagePublicKey() {
return (
<div className="grid-container margin-bottom-5 tablet:margin-top-6">
Copy link

Choose a reason for hiding this comment

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

Another point is that react-uswds provides some atomic elements for things like the grid container, which I think we should start using despite the fact that they're very thin wrappers around modifier classes. Even if we move off of react-uswds, it'll be easier to make the migration to another library or our own component system if we have one consistent way of treating them now instead of modifier classes.

@etanb @jpandersen87 Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As long as USWDS' grid is just shortcuts to flexbox (and thus react-uswds' grid) it sounds fine to me.

victor-chaparro and others added 3 commits March 10, 2023 10:17
* Added ability to remove filtered conditions from a bundle

---------

Co-authored-by: Jessica Willis <jessicawillis@navapbc.com>
* Update to Node 18
Fixes #7078
- Changed <div> to <GridContainer>.
- Removed extra <div>
@penny-lischer penny-lischer requested a review from a team as a code owner March 10, 2023 18:17
@penny-lischer penny-lischer temporarily deployed to staging March 10, 2023 18:18 — with GitHub Actions Inactive
@github-actions
Copy link

Test Results

760 tests  +4   756 ✔️ +4   1m 19s ⏱️ +15s
  94 suites ±0       4 💤 ±0 
  94 files   ±0       0 ±0 

Results for commit 7d626ad. ± Comparison against base commit 216c0e1.

@snesm snesm temporarily deployed to staging March 10, 2023 18:29 — with GitHub Actions Inactive
Copy link

@stephenkao stephenkao left a comment

Choose a reason for hiding this comment

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

Just a small lint error, but this looks good to me!

- Fix lint error
@penny-lischer penny-lischer temporarily deployed to staging March 10, 2023 19:09 — with GitHub Actions Inactive
@sonarcloud
Copy link

sonarcloud bot commented Mar 10, 2023

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

33.3% 33.3% Coverage
0.0% 0.0% Duplication

@penny-lischer penny-lischer merged commit 25e4a79 into master Mar 10, 2023
@penny-lischer penny-lischer deleted the experience/8395/public-key-tool branch March 10, 2023 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experience Team label to flag issues owned by the Experience Team frontend Work Type label to flag work related to the front-end websites
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Manage Public Key] Add new route, and empty page
5 participants