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

1.Reduce space between question title & options & Make spacing a configurable attribute #1986

Conversation

khyativyasargus
Copy link
Contributor

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #1967

Description

  1. Reduce space between question title & options so that the default spacing to be 4dp (so it is consistent with a 4dp grid)
  2. Make spacing a configurable attribute

Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

Type
Choose one: Feature

Screenshots (if applicable)

Checklist

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

2.Make spacing a configurable attribute
@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented Apr 25, 2023

@shelaghm @santosh-pingle

Before SS (16dp padding between each question title and options)

image

After default padding to 4dp SS

image

@shelaghm
Copy link

Thank you, @khyativyasargus Such a good improvement. To my eyes, the 4dp still looks like it is more than 4dp. The 'Gender' padding seems smaller than the other ones? Is there padding on other elements which is making it appear to be more than 4dp? Thanks for working on this issue.

@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented Apr 26, 2023

Hi @shelaghm Thanks for inputs!

> To my eyes, the 4dp still looks like it is more than 4dp. The 'Gender' padding seems smaller than the other ones? Is there padding on other elements which is making it appear to be more than 4dp?

I think the screenshots I attached here is in bigger size visually that's why you might be feeling like it's more than 4dp. Let me share proper ss again and let me know on that.
and for Gender widget I'll check today what's causing it to be different than others.

image

@santosh-pingle
Copy link
Collaborator

@khyativyasargus
Can you please also add screenshots for changes when instructions text are also present.

If you see default layout in the catalog app, it has instructions just below the question text.

@santosh-pingle
Copy link
Collaborator

@khyativyasargus any update on this PR

@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented Jul 6, 2023

@khyativyasargus Can you please also add screenshots for changes when instructions text are also present.

If you see default layout in the catalog app, it has instructions just below the question text.

image
image
image

@khyativyasargus
Copy link
Contributor Author

khyativyasargus commented Jul 6, 2023

@santosh-pingle As Shelaghm observed, Questions with EditTextView as answer view having more space than others due to EditText default style outer space, you can notice in screenshots I shared.

@khyativyasargus khyativyasargus requested a review from a team as a code owner July 6, 2023 06:15
@jingtang10 jingtang10 enabled auto-merge (squash) July 7, 2023 10:42
auto-merge was automatically disabled July 7, 2023 16:10

Head branch was pushed to by a user without write access

datacapture/src/main/res/values/attrs.xml Outdated Show resolved Hide resolved
datacapture/src/main/res/values/styles.xml Outdated Show resolved Hide resolved
datacapture/src/main/res/values/styles.xml Outdated Show resolved Hide resolved
@jingtang10 jingtang10 enabled auto-merge (squash) July 7, 2023 17:38
@jingtang10 jingtang10 merged commit 9cbc1f4 into google:master Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

[SDC] Reduce space between question title & options in default layout
5 participants