Skip to content

LG-3840 Updated welcome and new agreement pages#4811

Merged
stevegsa merged 28 commits intomainfrom
stevegsa-new-welcome-screen
Mar 25, 2021
Merged

LG-3840 Updated welcome and new agreement pages#4811
stevegsa merged 28 commits intomainfrom
stevegsa-new-welcome-screen

Conversation

@stevegsa
Copy link
Contributor

@stevegsa stevegsa commented Mar 19, 2021

Welcome screen

Screen Shot 2021-03-19 at 10 12 22 PM

Screen Shot 2021-03-19 at 10 12 39 PM

Agreement screen

Screen Shot 2021-03-19 at 10 16 33 PM

Screen Shot 2021-03-19 at 10 16 47 PM

@stevegsa stevegsa marked this pull request as ready for review March 19, 2021 05:25
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Looks like the fourth bullet item is breaking onto a new line:

image

@zachmargolis
Copy link
Contributor

Looks like the fourth bullet item is breaking onto a new line:

Thanks for checking, @aduth ! @stevegsa, next time there are visual changes in a PR can you help us include a screenshot for context?

@stevegsa
Copy link
Contributor Author

Sure thing but I never marked it ready for review :-)

@aduth
Copy link
Contributor

aduth commented Mar 19, 2021

My mistake, I was going by

image

I'll check for the label next time

@stevegsa
Copy link
Contributor Author

No worries I took it out of draft optimistically this morning thinking the label was forthcoming...my bad

@stevegsa
Copy link
Contributor Author

@stevegsa
Copy link
Contributor Author

I'm going to convert that dash to an em dash

@stevegsa
Copy link
Contributor Author

Spacing needs some tweaks too

@stevegsa stevegsa requested a review from Danielle-Lee March 19, 2021 20:30
Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

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

Thanks, screenshots are looking pretty good! @stevegsa I have some questions about the headings and list items styles, but it's hard to tell without previewing it, so I've set up a couple minutes for later today to walk through it (feel free to move around if there's another time you prefer)

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM, some small suggestions, also want to make sure we get @anniehirshman-gsa 's signoff

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Should fix up the Spanish heading, but otherwise LGTM 👍

Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

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

To recap yesterday's sync:

  1. Adjusted visual style of h1s (kept consistent with current h1s on other pages) and list items (same size on mobile and desktop).
  2. Adjusted spacing of list items so they align with the blue numbers.

And now LGTM! I will investigate h1 heading size and required field behavior separately from this ticket (outside of scope).

@amathews-fs
Copy link
Contributor

@stevegsa Just to make sure I'm getting this correctly:

This is a new step in the FSM between Welcome and Upload all the normal FSM logged events should appear as expected and will not effect the log events for either Welcome or Upload?

@zachmargolis
Copy link
Contributor

@stevegsa Just to make sure I'm getting this correctly:

This is a new step in the FSM between Welcome and Upload all the normal FSM logged events should appear as expected and will not effect the log events for either Welcome or Upload?

Random thought, we do we have any specs that ensure all the events we reference in the funnel dashboard get logged (ex so we can catch regressions that would affect the funnel dashboard before they merge?)

@amathews-fs
Copy link
Contributor

Random thought, we do we have any specs that ensure all the events we reference in the funnel dashboard get logged (ex so we can catch regressions that would affect the funnel dashboard before they merge?)

@zachmargolis I don't think we have anything really centralized or assured coverage for that no. There are tests that do expect some events but it hasn't been something consistently done in the past. It's a good idea. I brought it up today in sprint review to look at spending some time making a IAL2 logging test suite to assure we at least don't have regressions.

@stevegsa
Copy link
Contributor Author

@stevegsa Just to make sure I'm getting this correctly:

This is a new step in the FSM between Welcome and Upload all the normal FSM logged events should appear as expected and will not effect the log events for either Welcome or Upload?

That is correct.

@stevegsa stevegsa merged commit 24dcf62 into main Mar 25, 2021
@stevegsa stevegsa deleted the stevegsa-new-welcome-screen branch March 25, 2021 04:11
zachmargolis added a commit that referenced this pull request Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants