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

CRDCDH-2086 Add OMB/PANS banner #560

Merged
merged 3 commits into from
Dec 11, 2024
Merged

CRDCDH-2086 Add OMB/PANS banner #560

merged 3 commits into from
Dec 11, 2024

Conversation

amattu2
Copy link
Member

@amattu2 amattu2 commented Dec 10, 2024

Overview

This PR adds a OMB banner to the Submission Request page, before the first question.

Note

There is some slight styling differences between the implementation and design. In particular, the page banner background cuts off much higher up, rather than fading out. Since it's not directly related to this change, I left it untouched.

I also noticed that the sidebar (ProgressBar) no longer follows the top of the screen when scrolling, even though the styling for it has it set to position: sticky. Unsure if this is expected or when it broke, but I left it untouched.

Note

I wanted to limit the changes since we're close to the 3.1.0 release, but in the future, I think we should migrate the FormContainer "Return to All Submissions" to a prefixElement instead (defined within the Review page). Just something to consider.

Change Details (Specifics)

  • Add OMB (official name is Privacy Act Notification Statement) banner to the Submission Request Section A
  • Add related tests for testable requirements (approval number and expiration date)

Related Ticket(s)

CRDCDH-2086 (FE Task)
CRDCDH-2085 (Design, Option 5)
CRDCDH-2068 (US)

@amattu2 amattu2 added this to the 3.1.0 (PMVP-M2) milestone Dec 10, 2024
@coveralls
Copy link
Collaborator

coveralls commented Dec 10, 2024

Pull Request Test Coverage Report for Build 12279370412

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 54.222%

Totals Coverage Status
Change from base Build 12279179888: 0.03%
Covered Lines: 3768
Relevant Lines: 6455

💛 - Coveralls

@amattu2 amattu2 marked this pull request as ready for review December 11, 2024 15:28
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega left a comment

Choose a reason for hiding this comment

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

Won't be addressed in this PR, but regarding the sticky sidebar no longer working, it seems to have been broken for a very long time. Unsure if removing the overflow causes issues else-where, but it seems to be the cause.

// src/layouts/index.tsx

const StyledWrapper = styled("main")({
  minHeight: "400px",
  overflowX: "hidden", // Removing this fixes the "position: sticky" sidebar
});

Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega left a comment

Choose a reason for hiding this comment

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

LGTM! Works well. We should definitely address the problems you found in 3.2.0.

@Alejandro-Vega Alejandro-Vega merged commit b4c5c17 into 3.1.0 Dec 11, 2024
7 checks passed
@Alejandro-Vega Alejandro-Vega deleted the CRDCDH-2086 branch December 11, 2024 17: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.

3 participants