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

[View Institution Profile] Language updates #1024

Merged
merged 14 commits into from
Nov 5, 2024

Conversation

meissadia
Copy link
Collaborator

@meissadia meissadia commented Oct 22, 2024

Closes #1013

Completed Changes

  • Updated the fallback values for most fields to "Not applicable"
  • Ensured Type of Financial Institution, Federal Taxpayer Identification Number (TIN) show as "Not provided" when missing
  • Updated language for LEI Status from "Active/Inactive" to "Issued/Lapsed"
  • Display alert for Type of Financial Institution, Federal Taxpayer Identification Number (TIN), or LEI Status when missing/lapsed.

How to test this PR

Language updates

    const testBlankValues = {
      lei: 'FI_LEI',
      name: 'FI_INSTITUTION_NAME',
      hq_address_street_1: 'HQ_ADDRESS_STREET_1',
      hq_address_city: 'HQ_ADDRESS_CITY',
      hq_address_state: 'HQ_ADDRESS_STATE',
      hq_address_state_code: 'HQ_ADDRESS_STATE_CODE',
      hq_address_zip: 'HQ_ADDRESS_ZIP',
    };
  1. [ViewInstitutionProfile > index.tsx] Replace the data param highlighted in the screenshot below with the following.
    Screenshot 2024-10-22 at 2 03 45 PM

  2. Visit the Institution profile page

  3. Confirm fields display the correct fallback value (blank or "Not applicable").

Screenshots

Language updates

  • Fields marked as Always populated in the reference table are populated with their fieldname (i.e FI_INSTITUTION_NAME)
  • Field that were previously "blank" are displayed as "Not populated" (TIN, Types of Financial Institution).
  • Others are populated with the appropriate fallback value.

screencapture-localhost-8899-institution-123456789TESTBANK123-2024-11-01-10_20_28

@meissadia meissadia force-pushed the 1013-institution_profile-language_and_error branch from b6a5d59 to 9fe3971 Compare October 28, 2024 19:20
@meissadia meissadia marked this pull request as ready for review October 28, 2024 19:22
@meissadia meissadia changed the title [View Institution Profile] Language updates, error handling [View Institution Profile] Language updates Oct 28, 2024
tanner-ricks
tanner-ricks previously approved these changes Oct 30, 2024
Copy link
Contributor

@tanner-ricks tanner-ricks left a comment

Choose a reason for hiding this comment

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

Looks fine from my end. Don't know if Natalia needs to check this out or not though.

@meissadia
Copy link
Collaborator Author

@natalia-fitzgerald I've updated this PR to switch blank fields to display as "Not provided". The screenshot above reflects these changes.

@meissadia
Copy link
Collaborator Author

@natalia-fitzgerald Added the warning alert when missing TIN or Type of FI. Screenshot above has been updated.

I notice the spacing between ! icon and the text in my implementation compared to your reference image differs (less in my implementation). Wondering if this is a one-off issue we should address in the SBL Frontend or if this is an issue with the DSR's Alert (field-level) component.

@meissadia meissadia force-pushed the 1013-institution_profile-language_and_error branch from 1a0537d to d2a96ea Compare October 31, 2024 21:55
@meissadia
Copy link
Collaborator Author

@natalia-fitzgerald Also updated to include the alert for Lapsed LEIs.

Copy link

@natalia-fitzgerald natalia-fitzgerald left a comment

Choose a reason for hiding this comment

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

@meissadia
This is looking good. Just one text adjustment and we should be good to go on the immediate task.

For "Type of financial institution" can you update the text to:
You must provide your type of financial institution to file. Visit update your financial institution profile to provide this information.


I also noticed the spacing/alignment issue between ! icon and the text. I seem to recall us flagging this as a problem with the DSR's field level alert.

Here's how the DSR's field level alert looks:

  • Space between icon and text is tight and text appears to sit on the baseline of the icon (rather than centered)
Screenshot 2024-10-31 at 9 00 52 PM

This might be the PR where we explored fixing this (almost a year ago):
cfpb/design-system-react#278

@meissadia
Copy link
Collaborator Author

@natalia-fitzgerald Alert message for Type of financial institution has been updated. Screenshot updated.

Based on the referenced PR about Alert spacing, sounds like a "later" item, so I'll leave it alone for now.

Copy link

@natalia-fitzgerald natalia-fitzgerald left a comment

Choose a reason for hiding this comment

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

@meissadia
This looks good.

My only remaining question is whether email domain should be "not available" as shown in the screenshot. My understanding is that we would always have email domain unless the entire system is down in some way. Is this correct? If so, I don't think we would ever show "Not available" for that field.

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 @meissadia!

@meissadia meissadia merged commit a697d31 into main Nov 5, 2024
4 checks passed
@meissadia meissadia deleted the 1013-institution_profile-language_and_error branch November 5, 2024 19:18
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.

[View Institution profile] Update language
4 participants