From 5da4d5e808ed8d300b8886ba1d19c0ab2c632ea7 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Thu, 20 Apr 2023 15:06:21 +0800 Subject: Material: fix clipped floating placeholder text The changes to placeholder text in 20e3d1b522d1b79239e9ac4a6af47ce3648512bd and 5704741a4073ac131782c3dd73cac5cda6800a28 result in floating placeholder text (i.e. the text shown at the top of the control when it has active focus) being clipped when e.g. in a ScrollView. This is probably only an issue for TextArea in practice, since you wouldn't usually put a TextField in a ScrollView, but you can still run into it if you clip it. We don't want to unconditionally set topInset by default, because, as mentioned above, these controls are not always clipped. In most cases they are used on their own, and this issue won't affect them. Unconditionally setting topInset would ruin the layout of existing UIs. So, we set topInset only if the control itself clips (or its Flickable parent in the case of TextArea). [ChangeLog][Controls][Material] The outlined TextArea now sets topInset by default if it or its Flickable parent clips. This avoids the floating placeholder being clipped in those cases. The outlined TextField sets topInset by default only if the TextField itself clips. Fixes: QTBUG-113321 Pick-to: 6.5 Change-Id: I8555e4fc0c7a9800f76b54a84d94f4d04691bc23 Reviewed-by: Richard Moe Gustavsen --- .../doc/src/qtquickcontrols-material.qdoc | 21 +++++++ src/quickcontrols/material/TextArea.qml | 7 ++- src/quickcontrols/material/TextField.qml | 7 ++- .../impl/qquickmaterialplaceholdertext.cpp | 19 ++++++- .../qquickmaterialstyle/data/tst_material.qml | 64 ++++++++++++++++++++++ 5 files changed, 114 insertions(+), 4 deletions(-) diff --git a/src/quickcontrols/doc/src/qtquickcontrols-material.qdoc b/src/quickcontrols/doc/src/qtquickcontrols-material.qdoc index 0bfb2bed80..7dd21c29ac 100644 --- a/src/quickcontrols/doc/src/qtquickcontrols-material.qdoc +++ b/src/quickcontrols/doc/src/qtquickcontrols-material.qdoc @@ -221,6 +221,27 @@ Note that the heights shown above may vary based on differences in fonts across platforms. + \section2 Control-Specific Notes + + \target material-control-specific-notes-textarea + \section3 TextArea + + TextArea supports two + \l {material-containerStyle-attached-prop}{containerStyles}: \c Filled and + \c Outlined. Outlined TextAreas have floating placeholder text that sits at + the top of the control. This requires the placeholder text to go outside + the bounds of the control, which can cause it to be + clipped when the TextArea or the Flickable it's a child of sets + \l {Item::}{clip} to \c true. To avoid this, \l {Control::}{topInset} is + set to an appropriate value in these cases. + + \section3 TextField + + The same \l {material-control-specific-notes-textarea}{issue with clipping} + explained above for TextArea can also occur with \l TextField. To avoid + this, \l {Control::}{topInset} is set to an appropriate value when the + TextField sets clip to \c true. + \section1 Attached Property Documentation \styleproperty {Material.accent} {color} {material-accent-attached-prop} diff --git a/src/quickcontrols/material/TextArea.qml b/src/quickcontrols/material/TextArea.qml index 58eef10231..4a975295a5 100644 --- a/src/quickcontrols/material/TextArea.qml +++ b/src/quickcontrols/material/TextArea.qml @@ -16,6 +16,10 @@ T.TextArea { implicitHeight: Math.max(contentHeight + topPadding + bottomPadding, implicitBackgroundHeight + topInset + bottomInset) + // If we're clipped, or we're in a Flickable that's clipped, set our topInset + // to half the height of the placeholder text to avoid it being clipped. + topInset: clip || (parent?.parent as Flickable && parent?.parent.clip) ? placeholder.largestHeight / 2 : 0 + leftPadding: Material.textFieldHorizontalPadding rightPadding: Material.textFieldHorizontalPadding // Need to account for the placeholder text when it's sitting on top. @@ -23,7 +27,8 @@ T.TextArea { ? Material.textFieldVerticalPadding + placeholder.largestHeight // When the condition above is not met, the text should always sit in the middle // of a default-height TextArea, which is just near the top for a higher-than-default one. - : (implicitBackgroundHeight - placeholder.largestHeight) / 2 + // Account for any topInset as well, otherwise the text will be too close to the background. + : ((implicitBackgroundHeight - placeholder.largestHeight) / 2) + topInset bottomPadding: Material.textFieldVerticalPadding color: enabled ? Material.foreground : Material.hintTextColor diff --git a/src/quickcontrols/material/TextField.qml b/src/quickcontrols/material/TextField.qml index 97174de188..d29b247ce1 100644 --- a/src/quickcontrols/material/TextField.qml +++ b/src/quickcontrols/material/TextField.qml @@ -15,6 +15,9 @@ T.TextField { implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset, contentHeight + topPadding + bottomPadding) + // If we're clipped, set topInset to half the height of the placeholder text to avoid it being clipped. + topInset: clip ? placeholder.largestHeight / 2 : 0 + leftPadding: Material.textFieldHorizontalPadding rightPadding: Material.textFieldHorizontalPadding // Need to account for the placeholder text when it's sitting on top. @@ -22,7 +25,9 @@ T.TextField { ? placeholderText.length > 0 && (activeFocus || length > 0) ? Material.textFieldVerticalPadding + placeholder.largestHeight : Material.textFieldVerticalPadding - : Material.textFieldVerticalPadding + // Account for any topInset (used to avoid floating placeholder text being clipped), + // otherwise the text will be too close to the background. + : Material.textFieldVerticalPadding + topInset bottomPadding: Material.textFieldVerticalPadding color: enabled ? Material.foreground : Material.hintTextColor diff --git a/src/quickcontrols/material/impl/qquickmaterialplaceholdertext.cpp b/src/quickcontrols/material/impl/qquickmaterialplaceholdertext.cpp index 7ec3386457..8533802a5a 100644 --- a/src/quickcontrols/material/impl/qquickmaterialplaceholdertext.cpp +++ b/src/quickcontrols/material/impl/qquickmaterialplaceholdertext.cpp @@ -10,6 +10,7 @@ #include #include #include +#include QT_BEGIN_NAMESPACE @@ -106,6 +107,17 @@ void QQuickMaterialPlaceholderText::updateY() setY(shouldFloat() ? floatingTargetY() : normalTargetY()); } +qreal controlTopInset(QQuickItem *textControl) +{ + if (const auto textArea = qobject_cast(textControl)) + return textArea->topInset(); + + if (const auto textField = qobject_cast(textControl)) + return textField->topInset(); + + return 0; +} + qreal QQuickMaterialPlaceholderText::normalTargetY() const { auto *textArea = qobject_cast(textControl()); @@ -115,7 +127,10 @@ qreal QQuickMaterialPlaceholderText::normalTargetY() const // (one-line) if its explicit height is greater than or equal to its // implicit height - i.e. if it has room for it. If it doesn't have // room, just do what TextField does. - return (m_controlImplicitBackgroundHeight - m_largestHeight) / 2.0; + // We should also account for any topInset the user might have specified, + // which is useful to ensure that the text doesn't get clipped. + return ((m_controlImplicitBackgroundHeight - m_largestHeight) / 2.0) + + controlTopInset(textControl()); } // When the placeholder text shouldn't float, it should sit in the middle of the TextField. @@ -131,7 +146,7 @@ qreal QQuickMaterialPlaceholderText::floatingTargetY() const // Outlined text fields have the placeaholder vertically centered // along the outline at the top. - return -m_largestHeight / 2; + return (-m_largestHeight / 2) + controlTopInset(textControl()); } /*! diff --git a/tests/auto/quickcontrols/qquickmaterialstyle/data/tst_material.qml b/tests/auto/quickcontrols/qquickmaterialstyle/data/tst_material.qml index 8e4189924f..f77c0c5fc8 100644 --- a/tests/auto/quickcontrols/qquickmaterialstyle/data/tst_material.qml +++ b/tests/auto/quickcontrols/qquickmaterialstyle/data/tst_material.qml @@ -1082,4 +1082,68 @@ TestCase { verify(textContainer as MaterialImpl.MaterialTextContainer) compare(textContainer.focusAnimationProgress, 0) } + + function test_outlinedTextAreaInFlickablePlaceholderTextClipping() { + let flickable = createTemporaryObject(flickableTextAreaComponent, testCase) + verify(flickable) + + let textArea = flickable.TextArea.flickable + verify(textArea) + let placeholderTextItem = flickable.children[2] + verify(placeholderTextItem as MaterialImpl.FloatingPlaceholderText) + compare(textArea.Material.containerStyle, Material.Outlined) + // The Flickable doesn't clip at the moment so topInset should be 0. + compare(textArea.topInset, 0) + compare(textArea.topPadding, (textArea.implicitBackgroundHeight - placeholderTextItem.largestHeight) / 2) + + // topInset should now be half the placeholder text's height, + // and topPadding adjusted accordingly. + flickable.clip = true + compare(textArea.topInset, placeholderTextItem.largestHeight / 2) + compare(textArea.topPadding, ((textArea.implicitBackgroundHeight - placeholderTextItem.largestHeight) / 2) + textArea.topInset) + + // When the text is cleared, the placeholder text shouldn't float, but it should still be accounted for + // to avoid it causing jumps in layout sizes, for example. + const initialText = textArea.text + textArea.text = "" + compare(textArea.topPadding, ((textArea.implicitBackgroundHeight - placeholderTextItem.largestHeight) / 2) + textArea.topInset) + + flickable.clip = false + compare(textArea.topInset, 0) + compare(textArea.topPadding, (textArea.implicitBackgroundHeight - placeholderTextItem.largestHeight) / 2) + } + + function test_outlinedTextAreaPlaceholderTextClipping() { + let textArea = createTemporaryObject(textAreaComponent, testCase, { + placeholderText: "Type something", + text: "Text" + }) + verify(textArea) + let placeholderTextItem = textArea.children[0] + verify(placeholderTextItem as MaterialImpl.FloatingPlaceholderText) + compare(textArea.topInset, 0) + compare(textArea.topPadding, (textArea.implicitBackgroundHeight - placeholderTextItem.largestHeight) / 2) + + // topInset should now be half the placeholder text's height, and topPadding adjusted accordingly. + textArea.clip = true + compare(textArea.topInset, placeholderTextItem.largestHeight / 2) + compare(textArea.topPadding, ((textArea.implicitBackgroundHeight - placeholderTextItem.largestHeight) / 2) + textArea.topInset) + } + + function test_outlinedTextFieldPlaceholderTextClipping() { + let textField = createTemporaryObject(textFieldComponent, testCase, { + placeholderText: "Type something", + text: "Text" + }) + verify(textField) + let placeholderTextItem = textField.children[0] + verify(placeholderTextItem as MaterialImpl.FloatingPlaceholderText) + compare(textField.topInset, 0) + compare(textField.topPadding, textField.Material.textFieldVerticalPadding) + + // topInset should now be half the placeholder text's height, and topPadding adjusted accordingly. + textField.clip = true + compare(textField.topInset, placeholderTextItem.largestHeight / 2) + compare(textField.topPadding, textField.Material.textFieldVerticalPadding + textField.topInset) + } } -- cgit v1.2.1