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

feat: [InstitutionHeading] Support rendering as a link (isProfileLink) to the institution profile #1021

Closed
wants to merge 3 commits into from

Conversation

meissadia
Copy link
Collaborator

@meissadia meissadia commented Oct 18, 2024

Closes #1012

Changes

  • feat: InstitutionHeading: Added prop isProfileLink to determine if the heading should render as a link to the Institution profile
  • task: Filing app pages that use InstitutionHeading: Provide LEI so that we can link to the Institution profile

How to test this PR

  1. Run through the filing process
  2. On each page of the process, confirm the Institution heading's link redirects to the Institution profile

Screenshots

Unvisited Visited
Screenshot 2024-10-18 at 11 12 06 AM Screenshot 2024-10-18 at 11 12 17 AM

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.

I think we'll need to underline these institution links for accessibility? @natalia-fitzgerald

@natalia-fitzgerald
Copy link

@meissadia
Thanks for sharing the screenshots. This has helped to clarify some additional aspects of changing this to a link that I hadn't yet considered. I agree that ideally we would add an underline for accessibility and link consistency BUT adding the blue color and particularly the underline will affect the design. There's already an underline line (horizontal line) balancing act going on.

Can we pause on this work of making the financial institution name clickable? I need to think through the addition of the underline. And, since this work isn't tied to our sprint goals I'd like to take some time to discuss it with UX.

@meissadia meissadia marked this pull request as draft October 22, 2024 16:15
@meissadia
Copy link
Collaborator Author

Closing PR but keeping the unmerged branch as a future reference for #1012

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.

[Filing] Institution name links to Institution profile
3 participants