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

Fix #430 : Reduce white space between feedback & answer & content boxes #447

Merged
merged 6 commits into from
Nov 20, 2019

Conversation

veena14cs
Copy link
Contributor

@veena14cs veena14cs commented Nov 20, 2019

Explanation

This PR fixes white spaces between Feedback, answer and content boxes.

Note : Lerners's response box width based on its content is being fixed in #412 . This PR focuses only on spacing.

Mock

https://xd.adobe.com/spec/e2239cf4-9cde-4c08-5296-25316c1f0a14-9412/screen/5954b83a-4bcf-4fa6-8382-f814137fa8ab/PM-Q4-Fill-In-Learn-Again-/

Accessibility Scanner

  • Passes Scanner test successfully.

Screenshot

Screenshot_20191120-132734
Screenshot_20191120-132741

@seanlip
Copy link
Member

seanlip commented Nov 20, 2019

@veena14cs One question -- in the second screenshot, can you reduce the vertical whitespace at the top and the bottom of the item selection input? There's quite a lot of it.

Sorry I missed this earlier.

@seanlip seanlip assigned veena14cs and unassigned seanlip Nov 20, 2019
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@veena14cs LGTM, but it might get updated a little bit when we finish Hifi for exploration/state fragment. For now this should get merged.

@rt4914 rt4914 removed their assignment Nov 20, 2019
@veena14cs
Copy link
Contributor Author

@veena14cs LGTM, but it might get updated a little bit when we finish Hifi for exploration/state fragment. For now this should get merged.

Thanks.

@veena14cs
Copy link
Contributor Author

@veena14cs One question -- in the second screenshot, can you reduce the vertical whitespace at the top and the bottom of the item selection input? There's quite a lot of it.

Sorry I missed this earlier.

PTAL
Screenshot_20191120-143531

@veena14cs veena14cs assigned seanlip and unassigned veena14cs Nov 20, 2019
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. If it passes accessibility tests (please verify) then LGTM from a UI perspective.

@seanlip seanlip assigned veena14cs and unassigned seanlip Nov 20, 2019
@veena14cs
Copy link
Contributor Author

Thanks, looks good. If it passes accessibility tests (please verify) then LGTM from a UI perspective.

Yes it does. Thanks.

@veena14cs veena14cs merged commit 2fd729f into develop Nov 20, 2019
@veena14cs veena14cs deleted the reduce-white-space branch November 20, 2019 09:36
@mschanteltc
Copy link

Since this is a checkbox question, can we add a description above the choices that reads "Please select at least one choice" like in this mock? Also when the user submits the right answers, only their answers are shown, so any answers that were unselected should disappear as well.

These are the only functional improvements I would make to this, but otherwise the formatting looks great!

@nikitamarysolomanpvt nikitamarysolomanpvt removed their assignment Nov 26, 2019
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.

5 participants