Skip to content

[TextInputLayout] Add baseline alignment support #56

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

Closed
wants to merge 5 commits into from
Closed

[TextInputLayout] Add baseline alignment support #56

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
be6b361
[TextInputLayout] Add baseline alignment support
SUPERCILEX Jan 20, 2018
d549adb
Merge remote-tracking branch 'upstream/master' into patch-2
SUPERCILEX Apr 13, 2018
d0b2d5d
Merge branch 'master' into patch-2
SUPERCILEX Jul 9, 2018
fba6413
Address style concerns
SUPERCILEX Sep 4, 2018
12ddd96
Address review comments
SUPERCILEX Sep 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/java/com/google/android/material/textfield/TextInputLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,12 @@ public EditText getEditText() {
return editText;
}

@Override
public int getBaseline() {
EditText text = getEditText();
Copy link
Contributor

Choose a reason for hiding this comment

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

TextInputLayout already has an editText field. That field should be used instead.

return text == null ? super.getBaseline() : text.getPaddingTop() + text.getBaseline();
Copy link
Contributor

Choose a reason for hiding this comment

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

Returns statements in the codebase generally follow the opposite pattern for ordering null checks in ternary operations: editText != null ? ____ : ____ ;

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you verified that this works? getPaddingTop() alone won't provide the correct baseline, since TextInputLayout handles top spacing differently based in its boxBackgroundMode. calculateLabelMarginTop() is probably closer to what you want, although I don't think that will be enough to provide the correct baseline, either. You should also be aware that TextInputLayout adds spacing to the bottom of the box background, (boxBottomOffsetPx) which would throw off the calculation unless it's accounted for.

You should be able to get the correct baseline by offsetting the EditText's baseline by calculateLabelMarginTop(), getPaddingTop(), and the bottom offset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure all that is necessary? We've been using it for several months now in FirebaseUI without any problems. Also, both of these Stack Overflow questions found the same resolution.

If you think it won't work, would you mind explaining what boxBackgroundMode is and how I could activate it?

Example screenshot:
screenshot_20180904-105605

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, all that is necessary. This solution would help with aligning legacy text fields with other legacy text fields, because legacy text fields only add top padding to make room for the label. The text field boxes do other calculations in order to align properly, so these calculations need to be taken into account.

The currently proposed getBaseline() produces this effect when put in a LinearLayout with baselineAligned set to "true":

image

And this is the effect of a getBaseline() method that takes those factors into account:

image

The getBaseline() method in the screenshot above uses the following code:

  @Override
  public int getBaseline() {
    return editText != null
        ? editText.getBaseline() + calculateLabelMarginTop() + getPaddingTop() +
        : super.getBaseline();
  }

boxBottomOffsetPx should only be added if boxBackgroundMode != BOX_BACKGROUND_NONE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhh, I see. I haven't used the Material theme yet so I didn't notice that. Thanks for the great catch!

}

/**
* Set the hint to be displayed in the floating label, if enabled.
*
Expand Down