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

1910/Resolve issue with HouseholdSizeField #1991

Merged
merged 6 commits into from
Oct 20, 2021

Conversation

YazeedLoonat
Copy link
Collaborator

Issue

#1910

Addresses #issue

  • This change addresses the issue in full

Description

Resolves an issue where the HouseholdSizeField was displaying 0 when householdSizeMax was set to 0. Even though it shouldn't have displayed at all

Type of change

How Can This Be Tested/Reviewed?

If you have the ability to, create a listing where the house hold size max is set to 0, then test out the application process.
Once you get to the "Next we would like to know about the other people who will live with you in the unit" notice that a random 0 is NOT displayed on the page anymore

Checklist:

  • My code follows the style guidelines of this project
  • 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 updated the changelog to include a description of my changes
  • I have run yarn generate:client if I made backend changes
  • I have exported any new pieces in ui-components

@netlify
Copy link

netlify bot commented Oct 13, 2021

👷 Deploy request for dev-bloom pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: c34f38a

@netlify
Copy link

netlify bot commented Oct 13, 2021

👷 Deploy request for dev-partners-bloom pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: c34f38a

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

</>
)}
<span className="hidden">
<input
Copy link

Choose a reason for hiding this comment

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

Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

@netlify
Copy link

netlify bot commented Oct 13, 2021

✔️ Deploy Preview for dev-storybook-bloom ready!

🔨 Explore the source changes: c34f38a

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/616f17b04833bc00074ac21c

😎 Browse the preview: https://deploy-preview-1991--dev-storybook-bloom.netlify.app

@emilyjablonski
Copy link
Collaborator

Make sure you add the ready for review label when you want eyes on a PR :)

CHANGELOG.md Outdated
@@ -70,6 +70,7 @@ All notable changes to this project will be documented in this file. The format
- StandardTable styling bug ([#1632](https://github.com/bloom-housing/bloom/pull/1632)) (Emily Jablonski)
- More robust Features section for public listing view ([#1688](https://github.com/bloom-housing/bloom/pull/1688))
- A11Y issues with the image tint in ImageCard ([#1964](https://github.com/bloom-housing/bloom/pull/1964)) (Emily Jablonski)
- HouseholdSizeField bug when householdSizeMax is 0 ([#1910](https://github.com/bloom-housing/bloom/issues/1910)) (Yazeed)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be the PR number - look at the links above that use /pull

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

👏 You fixed the issue(s)! Great work.

@YazeedLoonat
Copy link
Collaborator Author

@emilyjablonski ty ty!
I got the changelog and accessibility updated and added the ready for review tag so should be okay for 👀 now

@emilyjablonski
Copy link
Collaborator

You've got some failing tests to look at :)

@seanmalbert
Copy link
Collaborator

@YazeedLoonat , see @emilyjablonski's comment on the failing tests.

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

👏 You fixed the issue(s)! Great work.

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

👏 You fixed the issue(s)! Great work.

@YazeedLoonat
Copy link
Collaborator Author

Hey @emilyjablonski from talking with sean and testing it out I'm pretty sure the remaining failure is related to #2024 and we may wanna spin up a separate ticketo to address it. Everything else should be good to go now

@emilyjablonski
Copy link
Collaborator

@YazeedLoonat How do I create a listing with a household max size of 0?

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.

LGTM on the previews!

@YazeedLoonat YazeedLoonat merged commit 71ded5e into bloom-housing:dev Oct 20, 2021
seanmalbert pushed a commit to CityOfDetroit/bloom that referenced this pull request Jun 23, 2022
* fix: householdSizeField hidden when householdSizeMax is 0
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