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

Prototype! #114

Merged
merged 18 commits into from
Jan 26, 2022
Merged

Prototype! #114

merged 18 commits into from
Jan 26, 2022

Conversation

KhalidAdan
Copy link
Contributor

@KhalidAdan KhalidAdan commented Jan 25, 2022

This PR incorporates all of the quick wins we spoke about in our meeting yesterday and removes the income field from the front page!! Woo!

I'm specifically looking for feedback on what you think of the sticky Need help div, is it positioned okay? (I moved it to the right so It wouldn't over lap the form but lmk what you think about positioning)

You can use the list below to test if you'd like!

  • add dynamic links on questions tab
  • "sticky" need help dialog on questions tab
  • on first page, put disclaimer before next button
  • income too high (disable results tab and button)
  • change field width to be consistent
  • other legal two lines
  • change back/clear/estimate
  • spacing between intro para and next buttons
  • add disclaimer (test site warning) at top of page (header)

Let me know where you land with your two outstanding items and if I need to do anything to help you move along!

@vercel
Copy link

vercel bot commented Jan 25, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

eligibility-estimator – ./

🔍 Inspect: https://vercel.com/eligibility-estimator/eligibility-estimator/A2D9D1ieyst1tKSDPEjacdeEVEXZ
✅ Preview: https://eligibility-estimator-git-ee-revie-2ab44b-eligibility-estimator.vercel.app

@JeremyKennedy
Copy link
Contributor

I think ideally when income is too high, that dialog comes from the backend. I think you could use what I provide now in the summary box to start, but I should probably update what I provide you so it provides text specifically related to income (currently it says "ineligible see below").

@KhalidAdan
Copy link
Contributor Author

KhalidAdan commented Jan 25, 2022

I think ideally when income is too high, that dialog comes from the backend. I think you could use what I provide now in the summary box to start, but I should probably update what I provide you so it provides text specifically related to income (currently it says "ineligible see below").

Good catch! I just copied and pasted the old text, I'll make the change now!

@JeremyKennedy
Copy link
Contributor

JeremyKennedy commented Jan 25, 2022

  • The spacing between the intro paragraph and the next button (first page) can probably be reduced @KhalidAdan
  • Update text in summary box when income too high, so that it says income is too high @JeremyKennedy
  • May need to add a new summary text property, one for when on the questions tab, one for on the results tab. This is because when we say "see below for more info" that is confusing when on the question tab. To discuss? Edit: Can hold off for now.

@KhalidAdan
Copy link
Contributor Author

KhalidAdan commented Jan 25, 2022

  • The spacing between the intro paragraph and the next button (first page) can probably be reduced @KhalidAdan

We can chat about this in our meeting at 2, it's like this in the designs :/

@JeremyKennedy
Copy link
Contributor

Regarding those two outstanding tasks from my comment - I'm probably going to skip those if I can't get them done today. I can't just go and change text without doing it "the right way" so I gotta wait for the incoming text changes to be provided to me. I really want to merge in our outstanding branches, and text changes can always be done later.

Copy link
Contributor

@JeremyKennedy JeremyKennedy left a comment

Choose a reason for hiding this comment

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

  • Changing page width between 991-992 increases the text size of the TEST SITE header, this seems unintentional as no other text size changes. Edit: this is hit-and-miss, this is probably related to the finicky issue we had earlier with Edge's responsive resizing. May be able to ignore this one, then. Edit2: Hmm, when I resize slowly, the text size changes, when I resize quickly, it stays. Maybe hitting a specific width is triggering something, so when I go fast it doesn't hit that specific width? Yeah... when I use the dimensions at the top rather than dragging, it's consistent.... mostly...
  • Related to above, when page width is specifically 992, the progress bar is messed up:
    image

To-do:

  • Ping the backend on page load to properly populate links and the Results More Info box? Or is this related to SSE and you maybe need to update the defaults? Not sure, I saw a network call to eligibility.json that appears to return the good stuff, so idk why it's not visible:
    image
  • Assuming the above is possible, remove the static contact SC link from your end
  • Unless the current design is approved with Alla, I'd lighten up the disabled elements
  • Make the Need Help floating box work more nicely on extra wide or extra narrow screens. Also, it's weird at width=992. It's fine with width=993. When width=992, it gets funky... it seems to not move at all? When width=991, it moves down to the bottom as intended for mobile. When width is really wide, like over 2000, it's too far to the right. This is most apparent on a 27" screen at 1440p (meaning this isn't some silly specific screen size like the 992 issues above).

components/Layout/NeedHelpList.tsx Outdated Show resolved Hide resolved
styles/globals.css Show resolved Hide resolved
pages/eligibility/index.tsx Outdated Show resolved Hide resolved
@KhalidAdan
Copy link
Contributor Author

  • Changing page width between 991-992 increases the text size of the TEST SITE header, this seems unintentional as no other text size changes. Edit: this is hit-and-miss, this is probably related to the finicky issue we had earlier with Edge's responsive resizing. May be able to ignore this one, then. Edit2: Hmm, when I resize slowly, the text size changes, when I resize quickly, it stays. Maybe hitting a specific width is triggering something, so when I go fast it doesn't hit that specific width? Yeah... when I use the dimensions at the top rather than dragging, it's consistent.... mostly...

This was actually code directly from SClabs, but I've removed the code for resizing text for consistency. If they want it back it's a very quick change.

  • Related to above, when page width is specifically 992, the progress bar is messed up:
    image

The useMediaQuery hook had a bug! It is now fixed but meant that 992 px was always a problem width for us 🤦🏾

To-do:

  • Ping the backend on page load to properly populate links and the Results More Info box? Or is this related to SSE and you maybe need to update the defaults? Not sure, I saw a network call to eligibility.json that appears to return the good stuff, so idk why it's not visible:

See comment below, fixed in ComponentFactory,tsx

  • Assuming the above is possible, remove the static contact SC link from your end
  • Unless the current design is approved with Alla, I'd lighten up the disabled elements
  • Make the Need Help floating box work more nicely on extra wide or extra narrow screens. Also, it's weird at width=992. It's fine with width=993. When width=992, it gets funky... it seems to not move at all? When width=991, it moves down to the bottom as intended for mobile. When width is really wide, like over 2000, it's too far to the right. This is most apparent on a 27" screen at 1440p (meaning this isn't some silly specific screen size like the 992 issues above).

All should be addressed!

Copy link
Contributor

@JeremyKennedy JeremyKennedy left a comment

Choose a reason for hiding this comment

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

All the fixes are great! Last thing is to make Need Help full width on mobile, but I'll pre-approve anyway!

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.

2 participants