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.

Sign up for GitHub

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

Jump to bottom

3032/new what to expect core #3087

Merged
merged 8 commits into from
Oct 12, 2022
Merged

3032/new what to expect core #3087

merged 8 commits into from
Oct 12, 2022

Conversation

jaredcwhite
Copy link
Collaborator

@jaredcwhite jaredcwhite commented Sep 24, 2022 •
edited
Loading

Issue Overview

This PR addresses #3032, #3033, #3034, and housingbayarea#492

  • This change addresses the issue in full
  • This change addresses only certain aspects of the issue
  • This change is a dependency for another issue
  • This change has a dependency from another issue

Description

This updates What to Expect-related text on the Get Started, Terms, and Confirmation form steps of public applications so they account for Lottery, FCFS, and Waitlist types.

Note: this also includes a first pass at a Gen2-ification of various Markdown styles (mostly ported over from HBA). Probably more cleanup would be good here, but feels out of scope for this particular issue.

How Can This Be Tested/Reviewed?

There are several listings of different types you can review here:
https://deploy-preview-3087--dev-bloom.netlify.app/listings

Checklist:

  • My code follows the style guidelines of this project
  • I have added QA notes to the issue with applicable URLs
  • I have performed a self-review of my own code
  • I have reviewed the changes in a desktop view
  • I have reviewed the changes in a mobile view
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have assigned reviewers
  • I have run yarn generate:client and/or created a migration if I made backend changes that require them
  • I have exported any new pieces added to ui-components
  • My commit message(s) is/are polished, and any breaking changes are indicated in the message and are well-described
  • Commits made across packages purposefully have the same commit message/version change, else are separated into different commits

Reviewer Notes:

Steps to review a PR:

  • Read and understand the issue, and ensure the author has added QA notes
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Also review the acceptance criteria on the Netlify deploy preview (noting that these do not yet include any backend changes made in the PR)
  • Either explicitly ask a clarifying question, request changes, or approve the PR if there are small remaining changes but the PR is otherwise good to go

On Merge:

If you have one commit and message, squash. If you need each message to be applied, rebase and merge.

@jaredcwhite jaredcwhite added the blocked Further development is blocked waiting for something external to this ticket label Sep 24, 2022
@netlify
Copy link

netlify bot commented Sep 24, 2022 •
edited
Loading

✅ Deploy Preview for dev-partners-bloom ready!

Name Link
🔨 Latest commit bf913b5
🔍 Latest deploy log https://app.netlify.com/sites/dev-partners-bloom/deploys/6345c0a0a77c8200081b05a6
😎 Deploy Preview https://deploy-preview-3087--dev-partners-bloom.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 24, 2022 •
edited
Loading

✅ Deploy Preview for dev-bloom ready!

Name Link
🔨 Latest commit bf913b5
🔍 Latest deploy log https://app.netlify.com/sites/dev-bloom/deploys/6345c0a02059a400085e2191
😎 Deploy Preview https://deploy-preview-3087--dev-bloom.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 24, 2022 •
edited
Loading

✅ Deploy Preview for dev-storybook-bloom ready!

Name Link
🔨 Latest commit bf913b5
🔍 Latest deploy log https://app.netlify.com/sites/dev-storybook-bloom/deploys/6345c0a08f66cd00084aaf68
😎 Deploy Preview https://deploy-preview-3087--dev-storybook-bloom.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@jaredcwhite jaredcwhite removed the blocked Further development is blocked waiting for something external to this ticket label Oct 4, 2022
@jaredcwhite jaredcwhite marked this pull request as ready for review October 4, 2022 17:48
@jaredcwhite jaredcwhite added the 2 reviews needed Requires 2 more review before ready to merge label Oct 4, 2022
emilyjablonski
emilyjablonski reviewed Oct 5, 2022
View reviewed changes
ui-components/src/global/markdown.scss
}
}

/* Used for a larger lead paragraph: */
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the future, let's separate out changes like this into a different PR so that we can make an issue, discuss it in planning, point it, shorten the review scope, and make sure that the point estimate we made for the initial issue in planning stays accurate :)

Copy link
Collaborator Author

@jaredcwhite jaredcwhite Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, I forgot to mention housingbayarea#492 in the PR description. It was something that had already come up with a previous review round…I didn't want to just port the Markdown styles over from HBA without at least modernizing it a bit, and I do think we should create another issue for further review of our Markdown styles (part of the audit?)

emilyjablonski
emilyjablonski requested changes Oct 5, 2022
View reviewed changes
Copy link
Collaborator

@emilyjablonski emilyjablonski left a comment

Choose a reason for hiding this comment

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

I have a few code comments, and then functionally all looks well except for a lottery listing confirmation page - I'm not seeing the preference string.

Screen Shot 2022-10-05 at 2 13 08 PM

@emilyjablonski emilyjablonski added needs changes The author must make changes and then re-request review before merging 1 review needed Requires 1 more review before ready to merge and removed 2 reviews needed Requires 2 more review before ready to merge labels Oct 5, 2022
@jaredcwhite
Copy link
Collaborator Author

@emilyjablonski I fixed the typos and switched to switch (ha!) statements. Regarding the preferences text, it's weird — the document seems to have blacked out those for the Confirmation screen, but not for the confirmation email:

image

I wonder if that was intentional or not?

@kathyccheng
Copy link
Collaborator

@jaredcwhite I think that was a mistake (the blacking out of preferences, sorry! I updated it. Let's add those in!

@jaredcwhite
Copy link
Collaborator Author

jaredcwhite commented Oct 11, 2022 •
edited
Loading

Changes made. Last round! (I hope)

emilyjablonski
emilyjablonski approved these changes Oct 11, 2022
View reviewed changes
Copy link
Collaborator

@emilyjablonski emilyjablonski left a comment

Choose a reason for hiding this comment

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

🥳 🥳 🥳

@emilyjablonski emilyjablonski removed the needs changes The author must make changes and then re-request review before merging label Oct 11, 2022
@ludtkemorgan ludtkemorgan added ready to merge Should be applied when a PR has been reviewed and approved and removed 1 review needed Requires 1 more review before ready to merge labels Oct 12, 2022
@jaredcwhite jaredcwhite merged commit f4e4a92 into dev Oct 12, 2022
ludtkemorgan pushed a commit to housingbayarea/bloom that referenced this pull request Oct 13, 2022
@jaredcwhite @ludtkemorgan
3032/new what to expect core (bloom-housing#3087)
8ece1f8 
* feat: bring over What to Expect and update Markdown styles

* feat: show dedicated content on the What to Expect form step

* feat: add additional text to terms and confirmation

* feat: update form conditionals to use waitlist enum

* fix: remove stray import

* fix: typos and improved switch statements

* fix: confirmation text and remove lottery date
ludtkemorgan added a commit to housingbayarea/bloom that referenced this pull request Oct 17, 2022
@ludtkemorgan @emilyjablonski @jaredcwhite
feat: backend changes for what to expect changes (#508)
85cb4da 
* fix: update email confirmation what to expect copy

* fix: email tests (bloom-housing#3095)

* feat: new listing type field

* 3032/new what to expect core (bloom-housing#3087)

* feat: bring over What to Expect and update Markdown styles

* feat: show dedicated content on the What to Expect form step

* feat: add additional text to terms and confirmation

* feat: update form conditionals to use waitlist enum

* fix: remove stray import

* fix: typos and improved switch statements

* fix: confirmation text and remove lottery date

* fix: add back in eligibility translations (bloom-housing#3134)

* fix: add back in detail page changes (#510)

* fix: remove check for whatToExpect in cypress test

Co-authored-by: Emily Jablonski <[email protected]>
Co-authored-by: Jared White <[email protected]>
ludtkemorgan pushed a commit that referenced this pull request Oct 25, 2022
@jaredcwhite @ludtkemorgan
3032/new what to expect core (#3087)
f73c877 
* feat: bring over What to Expect and update Markdown styles

* feat: show dedicated content on the What to Expect form step

* feat: add additional text to terms and confirmation

* feat: update form conditionals to use waitlist enum

* fix: remove stray import

* fix: typos and improved switch statements

* fix: confirmation text and remove lottery date
ludtkemorgan pushed a commit that referenced this pull request Oct 26, 2022
@jaredcwhite @ludtkemorgan
3032/new what to expect core (#3087)
85f47d7 
* feat: bring over What to Expect and update Markdown styles

* feat: show dedicated content on the What to Expect form step

* feat: add additional text to terms and confirmation

* feat: update form conditionals to use waitlist enum

* fix: remove stray import

* fix: typos and improved switch statements

* fix: confirmation text and remove lottery date
ludtkemorgan added a commit that referenced this pull request Oct 27, 2022
@ludtkemorgan @jaredcwhite
@emilyjablonski @lint-action @christine-sfg @KrissDrawing
feat: release 2022-10-25
2964d79 
* feat: new listing type field (#3085)

* 3032/new what to expect core (#3087)

* feat: bring over What to Expect and update Markdown styles

* feat: show dedicated content on the What to Expect form step

* feat: add additional text to terms and confirmation

* feat: update form conditionals to use waitlist enum

* fix: remove stray import

* fix: typos and improved switch statements

* fix: confirmation text and remove lottery date

* fix: add back in eligibility translations (#3134)

* fix: correct the cron jobs

* feat(expandablecontent): remove classname from ExpandableContent

move styling of more/less button to parent component

#3140

* Fix code style issues with Prettier

* feat(expandabletext): add classname to stories

add margin to instances of ExpandableText

#3140

* fix: switch listing to server side props (#3145)

* fix: update activity-log relationship to users for deletion (#3139)

* Allow markup for descriptions and optional classname (#3131)

* feat: display income currency value with commas

* feat: accept only numeric values in currency field

* feat: submit income as a string

* feat: remove redundant step prop from currency field

* feat: add thank you translations to email footer

* fix: change the cron job logging level (#3150)

* fix: what to expect missed changes

* fix: email tests

* chore(release): version

 - @bloom-housing/[email protected]
 - @bloom-housing/[email protected]
 - @bloom-housing/[email protected]
 - @bloom-housing/[email protected]
 - @bloom-housing/[email protected]

Co-authored-by: Jared White <[email protected]>
Co-authored-by: Emily Jablonski <[email protected]>
Co-authored-by: Mark Buckner <[email protected]>
Co-authored-by: Lint Action <[email protected]>
Co-authored-by: christine-sfg <[email protected]>
Co-authored-by: krzysztof ziecina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilyjablonski emilyjablonski emilyjablonski approved these changes

@ludtkemorgan ludtkemorgan ludtkemorgan approved these changes

@kramduckner kramduckner Awaiting requested review from kramduckner

@YazeedLoonat YazeedLoonat Awaiting requested review from YazeedLoonat

Assignees
No one assigned
Labels
ready to merge Should be applied when a PR has been reviewed and approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jaredcwhite @kathyccheng @ludtkemorgan @emilyjablonski