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

Make multi-line text widget taller by default #1517

Merged
merged 5 commits into from
Aug 2, 2022

Conversation

jingtang10
Copy link
Collaborator

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

Fixes #1197

Description
Set a minimum number of lines for the multi-line edit text widget.

The widet will initially have 3 lines to indicate that the widget itself supports multi-line input. But as the user types the widget would expand vertically to accommodate more lines.

Alternative(s) considered
NA

Type
Bug fix

Screenshots (if applicable)

multiline.mp4

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).

Copy link
Collaborator

@santosh-pingle santosh-pingle left a comment

Choose a reason for hiding this comment

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

Why do other 2 files shows code formatting changes?

LGTM, Thanks!

@jingtang10
Copy link
Collaborator Author

Why do other 2 files shows code formatting changes?

LGTM, Thanks!

i think probably other prs didn't run spotless properly? i'm not entirely sure

Comment on lines +42 to +46
style="?attr/questionnaireTextInputEditTextStyle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="top"
android:minLines="3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this change allow the developer application to change the minLines ? Maybe minLines should be part of a new multi-line text style?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks @aditya-07 - i think this should be probably done as a follow-up. We haven't heard any feature request about this one from the developers. Feel like this is a weaker requirement compared with what we still have to do to allow custom styles.

@santosh-pingle fyi @shelaghm fyi

@shelaghm
Copy link

@jingtang10 this looks good to me. It aligns with what I was thinking for paragraph text style.
Attaching screenshot for reference below of mocks I did previously

  1. One idea is to name this paragraph text input. Google forms uses this. What do you think?
  2. Do we want to add this as a separate component/widget or include in the existing text field component?

Screen Shot 2022-07-29 at 10 27 47

.

@jingtang10
Copy link
Collaborator Author

@jingtang10 this looks good to me. It aligns with what I was thinking for paragraph text style. Attaching screenshot for reference below of mocks I did previously

  1. One idea is to name this paragraph text input. Google forms uses this. What do you think?

Do you mean change the widget's name to Paragraph Text Input? If we do that we should also make sure the single line text input is renamed to Short answer text input or something similar.

As an engineer, I sort of prefer the more functional names i.e. single line and multi-line because you may have answers that are not a paragraph (e.g. bullet points?) but still multi line? or answers that are quite long... but fits in a single line? so i think these names are potentially more accurate.

But I do understand why semantically it might be easier for people to understand what these means... so I'm ok with short answer and paragraph if that's the preferred wording from a ux perspective.

  1. Do we want to add this as a separate component/widget or include in the existing text field component?

they are already separate components. but we're not showcasing them in the catalog app yet. i think the question here is wheather we put them in one single component or seprarate as two? both ok with me. single's probably fine and we can use the bottom sheet to toggle single/multi line?

Screen Shot 2022-07-29 at 10 27 47

.

Copy link
Contributor

@aditya-07 aditya-07 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jingtang10 jingtang10 merged commit 433ae36 into google:master Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Data capture - add support for the 'text' field type to render multiline text input
5 participants