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

content(Complete User Profile): Updated the language, commented out button #598

Merged
merged 15 commits into from
Jun 1, 2024

Conversation

shindigira
Copy link
Contributor

@shindigira shindigira commented May 29, 2024

closes #597

Note

Tasks - Complete your user profile (associated financial institutions)

  • Update small text in introduction to include link to email support staff and new URL for GLEIF. It would be best to swap in the whole block.

Tasks - Complete your user profile (no associated financial institutions)

  • Update small text in introduction to include link to email support staff and new URL for GLEIF. It would be best to swap in the whole block.
  • Update body text under "Provide your financial institution details" to reference emailing support staff instead of "Add an additional financial institution" button/link.
  • Change name of link/button to "Add an additional financial institution"
  • Comment out "Add an additional financial institution" button/link
  • For error alert links, remove numbering from "financial institution name" and "LEI" because there will only be one for MVP. You can comment out the numbering in code so that we can bring it back when we reveal the button/link post-MVP.
  • Update text on error alert for "Your email domain" is not authorized (pre-clearance change).
  • Add a breadcrumb to the warning alert notification screen that takes a user back to "Platform home" (un-authenticated).

Notable Changes

  • content(Complete User Profile): Updated the language, commented out button
  • feat(complete user profile): toggle showing key index per error -- currently disabled
  • enhancement(Summary): breadcrumb is standard for all summaries -- if authenticated, routes to /landing otherwise /

How to Test

  • Compare with the figma
  • Go through the two task lists above and verify each are correct; especially, last task.

Screenshot - Normal

Screenshot 2024-05-29 at 12 28 38 PM

Screenshot - With Errors

Screenshot 2024-05-29 at 12 28 47 PM

Screenshot - Error Summary with breadcrumb

Screenshot 2024-05-29 at 6 13 26 PM

Screenshot - Warning Summary with breadcrumb

Screenshot 2024-05-29 at 6 28 03 PM

References

#578

@shindigira shindigira marked this pull request as draft May 29, 2024 19:41
@shindigira shindigira marked this pull request as ready for review May 30, 2024 01:12
@natalia-fitzgerald
Copy link

natalia-fitzgerald commented May 30, 2024

@shindigira
I reviewed the changes. Just about everything else looks good.

Just one thing. Can you re-swap in the following text?

Complete your user profile (associated financial institutions)

  • Update small text in introduction to include link to email support staff and new URL for GLEIF. Swap in the whole block again.

Complete your user profile (no associated financial institutions)

  • Update small text in introduction to include link to email support staff and new URL for GLEIF. Swap in the whole block again.

Once these changes are live on AWS I will re-check and check off the list on the design review issue.

@shindigira
Copy link
Contributor Author

@natalia-fitzgerald Updated the PR. Will just need an approval and merge-in before putting on AWS

@natalia-fitzgerald
Copy link

@natalia-fitzgerald Updated the PR. Will just need an approval and merge-in before putting on AWS

@shindigira - Looks good to me!

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

Looks good! One minor nit, if you want to change it.

@@ -100,7 +103,9 @@ function FormErrorHeader<
: 'Missing entry'
}${
// eslint-disable-next-line @typescript-eslint/no-magic-numbers
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: I don't think you need this eslint exception now that you changed it to One?

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

Looks good!

@shindigira shindigira merged commit ff3eed6 into main Jun 1, 2024
4 checks passed
@billhimmelsbach billhimmelsbach deleted the 597-add-financial-institution-button-language-update branch January 15, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants