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

HCBS Footer 2/x - Set up app routes #13

Merged
merged 14 commits into from
Jul 31, 2024
Merged

HCBS Footer 2/x - Set up app routes #13

merged 14 commits into from
Jul 31, 2024

Conversation

ajaitasaini
Copy link
Contributor

@ajaitasaini ajaitasaini commented Jul 26, 2024

Description

Related ticket(s)

CMDCT-


How to test

Click the Contact us link in the footer and confirm it goes to our help page.

Follow up PRs will include: (potentially) re-organizing some of our styles and setting up similar routes for the Header

Important updates


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary

convert to a different template: test → val | val → prod

@ajaitasaini ajaitasaini changed the title Footer - app routes HCBS Footer 2/x - Set up app routes Jul 29, 2024
Copy link
Contributor

@benmartin-coforma benmartin-coforma left a comment

Choose a reason for hiding this comment

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

I don't see any glaring problems so far. I'd really love to be able to get by without importing date-fns or moment, but solving for that doesn't need to be a blocker for the Footer story.

services/ui-src/src/components/cards/EmailCard.tsx Outdated Show resolved Hide resolved
@ajaitasaini ajaitasaini marked this pull request as ready for review July 30, 2024 13:11
Copy link
Contributor

@rocio-desantiago rocio-desantiago left a comment

Choose a reason for hiding this comment

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

Double checking: Do we need the LoginCognito and LoginIDM for this PR?

Copy link
Contributor

@rocio-desantiago rocio-desantiago left a comment

Choose a reason for hiding this comment

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

When you merge the other Footer PR, will this testId get added back?

@ajaitasaini
Copy link
Contributor Author

When you merge the other Footer PR, will this testId get added back?

see last comment!

Copy link

codeclimate bot commented Jul 31, 2024

Code Climate has analyzed commit 7853cca and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 64.3% (90% is the threshold).

This pull request will bring the total coverage in the repository to 81.0% (-9.7% change).

View more on Code Climate.

Copy link
Contributor

@benmartin-coforma benmartin-coforma left a comment

Choose a reason for hiding this comment

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

Very nice!

@ajaitasaini ajaitasaini merged commit 04e526a into main Jul 31, 2024
15 of 17 checks passed
@ajaitasaini ajaitasaini deleted the footer-2 branch July 31, 2024 21:02
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.

3 participants