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

[TextInputLayout] TextInputLayout wrong minimum height #4146

Open
zhanghai opened this issue Apr 17, 2024 · 1 comment
Open

[TextInputLayout] TextInputLayout wrong minimum height #4146

zhanghai opened this issue Apr 17, 2024 · 1 comment

Comments

@zhanghai
Copy link

zhanghai commented Apr 17, 2024

Description: TextInputLayout.updateEditTextHeightBasedOnIcon() may set a wrong minimum height that lingers around. This is basically another way to reproduce #3451 where 4a2654a didn't fix the issue.

Expected behavior: TextInputLayout should resize with text.

Source code: The code snippet which is causing this issue

private boolean updateEditTextHeightBasedOnIcon() {
if (editText == null) {
return false;
}
// We need to make sure that the EditText's height is at least the same as the end or start
// icon's height (whichever is bigger). This ensures focus works properly, and there is no
// visual jump if the icon is enabled/disabled.
int maxIconHeight = Math.max(endLayout.getMeasuredHeight(), startLayout.getMeasuredHeight());
if (editText.getMeasuredHeight() < maxIconHeight) {
editText.setMinimumHeight(maxIconHeight);
return true;
}
return false;
}

It only ever sets the minimum height but doesn't reset it. And in case the EndCompoundLayout becomes GONE due to icon becoming hidden, the EndCompoundLayout won't participate in the next layout and thus EndCompoundLayout.getMeasuredHeight() would return a stale height.

The fix in 4a2654a used a TextWatcher to reset the minimum height, however it won't work if the EndCompoundLayout is becoming GONE after the text change, e.g. when the app removes the error state (and thus removing the error icon), before re-layout for the previous text change happened.

It seems to me the code in updateEditTextHeightBasedOnIcon() should inspect the visibility of EndCompoundLayout before using its measured height, and manually use 0 in the case EndCompoundLayout is GONE. It should also reset the minimum height when measure height of EndCompoundLayout is smaller and the minimum height of the edit text is different (this way we prevent loops).

Minimal sample app repro:

Something along the following lines:

textInputEditText.setText("a\nb\nc\nd")
textInputLayout.error = "error"
// Wait for layout and draw then in a separate place...
textInputEditText.setText("a")
textInputLayout.error = null

An actual example can be found at zhanghai/MaterialFiles@10a67dd and its APK artifact, by opening the file manager app, opening properties of a regular file and go to the checksum tab, put a multi-line inconsistent checksum (e.g. copy-pasting SHA-256 sum twice) into the "Compare" edit text and then clear the edit text.

Android API version: Android 14

Material Library version: 1.11.0 and 1.12.0-rc01

Device: Android Emulator Pixel 2 Android 14

@zhanghai zhanghai added the bug label Apr 17, 2024
zhanghai added a commit to zhanghai/MaterialFiles that referenced this issue Apr 17, 2024
This makes it visually better on phones, and also happens to work
around
material-components/material-components-android#4146
now that we don't have an end/error icon that's disappearing.
@paulfthomas
Copy link
Member

@pekingme could you take a look at this as you worked on 4a2654a thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants