#3481: Move alerts below breadcrumbs [meoward]

[kimallen]

Ticket

Resolves #3481

Changes

• Moves the alerts (error and success) to below the breadcrumbs rather than above for the following screens:
  • profile (same template for both org and non-org)
  • member:
    • portfolio_member
    • portfolio_member_domains
    • portfolio_member_domains_edit
    • portfolio_member_permissions
    • portfolio_members_add_new
  • domain:
    • domain_add_user
    • domain_nameservers
    • domain_security_email
    • domain_suborganization
    • domain_dns
    • domain_dnssec
    • domain-dsdata
    • domain_users

Context for reviewers

Because some of the alerts were in the domain_base.html so were appearing above the breadcrumbs, I added a breadcrumbs block before those alerts and then moved the breadcrumbs in the child templates into the content block (and out of the domain_content block)

Setup

In both the org model and non-org model, walk through each of the steps for creating a domain request. cause both success and error alerts and ensure they are below the breadcrumbs. Also be sure to go to a domain whose status is "DNS needed" and walk through the pages for adding DNS and check the breadcrumbs location.

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

• Met the acceptance criteria, or will meet them in a subsequent PR
• Created/modified automated tests
• Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

• If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
• Interactions with external systems are wrapped in try/except
• Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

• Tag @dotgov-designers in this PR's Reviewers for design review. If code is not user-facing, delete design reviewer checklist
• Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
• Checked keyboard navigability
• Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

• Pulled this branch locally and tested it
• Verified code meets all checks above. Address any checks that are not satisfied
• Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
• Checked that all code is adequately covered by tests
• Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

• New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
• Checked keyboard navigability
• Meets all designs and user flows provided by design/product
• Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
• (Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

• Checked that the design translated visually
• Checked behavior. Comment any found issues or broken flows.
• Checked different states (empty, one, some, error)
• Checked for landmarks, page heading structure, and links

Validated user-facing changes as a designer

• Checked keyboard navigability
• Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
• Tested with multiple browsers (check off which ones were used)
  • Chrome
  • Microsoft Edge
  • FireFox
  • Safari
• (Rarely needed) Tested as both an analyst and applicant user

References

• Code review best practices

Screenshots

[kimallen]

@erinysong - I reverted this change from a recent PR because for accessibility reasons, we should not remove main- it's always better to use semantic html. I didn't see how it impacted the styling, but I'm sure you made the change for a reason, so let me know if this undoes something. I'm sure we can find a way to make the desired change without removing the main tag.

[erinysong]

@kimallen thank you for changing that and explaining! We originally made that change because design noticed the grid sizing on the domain and domain request summary sidebars were different from the portfolio sidebar. We changed the domain and domain request layouts to match the portfolio but we can definitely revert this to maintain accessibility. cc: @gabo0oo who I was working on this during that PR's design review

[kimallen]

Thanks for the context @erinysong - I talked with @gabo0oo and figured out where the spacing issue was and believe that I adjusted it back while maintaining the <main> tag.

[witha-k]

Blocked from reviewing - running into 500 errors on the meoward sandbox when I try to access a domin in the admin portal & when I try to start domain requests in both org model and non-org model..

[witha-k]

Blocked from reviewing - running into 500 errors on the meoward sandbox when I try to access a domin in the admin portal & when I try to start domain requests in both org model and non-org model..

Never mind, some migrations weren't applied but Liz helped fix this issue!

[witha-k]

LGTM! 👍

[erinysong]

Thank you for moving all of these! This may be a separate bug but I noticed when prompting an alert on the domain renewal form tab, we have 2 breadcrumb menus so the alerts on this page appear twice above the breadcrumb. If updating this page and removing one of the breadcrumb menus ends up being more involved than a quickfix, happy to also make a bug ticket for this!

I believe expiring domains are a little hard to reproduce in sandboxes because they pull from registry data and I just happened to find this because @therealslimhsiehdy manually created an expiring domain for me in my sandbox for a past PR I was testing, so feel free to test on my sandbox by adding yourself as a domain manager for account-wind-plant.gov if that makes testing easier!

[erinysong]

Updated pages LGTM! Just had one change request commented above to update domain_renewal.html's error message placement that I think can be fixed by updating domain_renewal.html the same as you did the ones here

[kimallen]

Updated pages LGTM! Just had one change request commented above to update domain_renewal.html's error message placement that I think can be fixed by updating domain_renewal.html the same as you did the ones here

Good catch! And thanks for the tip on how to test it. I believe I have updated it, but I'm having trouble accessing the right things to view it in the sandbox. I sent an email to you with more details.

[erinysong]

Updated pages LGTM! Just had one change request commented above to update domain_renewal.html's error message placement that I think can be fixed by updating domain_renewal.html the same as you did the ones here

Good catch! And thanks for the tip on how to test it. I believe I have updated it, but I'm having trouble accessing the right things to view it in the sandbox. I sent an email to you with more details.

Thank you! Ah I don't think I got an email but happy to go over it from standup