-
Notifications
You must be signed in to change notification settings - Fork 24
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
fix: redirect and alert language #3467
Conversation
✅ Deploy Preview for bloom-demo-public ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for bloom-exygy-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
I think the language in the error message is good, and agree w your thought on the related bug. We can translate in google for now (see below) and I'll let Izzie know to send jurisdictions an FYI. Translations:
|
language looks good to me! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I think it might be useful to add it to a few more pages. Specifically the other ones in the "start" folder (autofill and what-to-expect). Also it might be a good idea to do the last page of the flow so the user doesn't get the error until after they submit. Thoughts?
@ludtkemorgan I wonder if we need to think more about a big picture solution before we start adding it to individual pages, I think ideally long term we just add this once, wondering if this can be our quick fix? |
Sorry to bug ya again @eajensenwa but just want to verify that the site alert styling/design looks good to you 🧲 |
yep, looks great! 🎨 |
I briefly spoke to colin in the huddle. I think it makes sense to not add it to all of the pages so I'm ok with just having it on the choose-language page. However, I do think we would get a lot of bang for our buck if we put it on the last page as well for this quick fix. That way it will catch everyone, even if they come from a different page since we don't know exactly how this is happening at the moment. The long term fix will require a bit more considerations |
@ludtkemorgan makes sense, sounds great! |
Updated @emilyjablonski @ludtkemorgan 📟 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⏰
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix: redirect and alert language * fix: translations added * fix: application summary coverage and translation handling * fix: test reset
* fix: stop transforming null to zero (bloom-housing#3453) * fix: stop transforming null to zero * fix: other fields defaulting to zero * fix: public multiple photos from detroit (bloom-housing#3390) * feat: add multiple photos * feat: add tests * feat: add images to seeds * fix: return array when no image * fix: shared helpers photos tests * fix: add class with variables to parent element * fix: make actions inside content section optional on desktop * fix: pass modalCloseInContent prop to listing view's ImageCard * fix: remove redundant image-card class * chore: upgrade ui-c * fix: remove redundant order column * fix: change font for drawer title * fix: remove default images value when editing * feat: adding app sub verification endpoint (bloom-housing#3452) * feat: adding app sub verification endpoint * fix: build fix * fix: redirect and alert language (bloom-housing#3467) * fix: redirect and alert language * fix: translations added * fix: application summary coverage and translation handling * fix: test reset --------- Co-authored-by: ColinBuyck <53269332+ColinBuyck@users.noreply.github.com> Co-authored-by: Krzysztof Zięcina <kziecina@airnauts.com> Co-authored-by: Yazeed Loonat <YazeedLoonat@gmail.com>
* fix: redirect and alert language * fix: translations added * fix: application summary coverage and translation handling * fix: test reset
Pull Request Template
Issue Overview
This PR addresses #3463
Description
This fix redirects a user who lands at the beginning of an application for a listing that is closed. Couple notes:
How Can This Be Tested/Reviewed?
This can be tested by pulling it down locally, setting a listing to closed, and then hitting this url:
http://localhost:3000/applications/start/choose-language?listingId= listingIdThatYouJustClosed
Checklist:
yarn generate:client
and/or created a migration if I made backend changes that require themReviewer Notes:
Steps to review a PR:
On Merge:
If you have one commit and message, squash. If you need each message to be applied, rebase and merge.