qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

QQuickScrollView and QQuickTextEdit: Fix binding loops 

If the content item of a ScrollArea is a height <-> width dependent
item, like a TextEdit, a change in the height of the content, may
trigger the vertical scrollbar to become visible, (if the content height
is bigger than the ScrollArea's height). This in turn causes the right
padding of the ScrollArea to change, to make place for the scrollbar,
causing in this way a content resize again, giving us a binding loop
on the Scrollbar's visible property and the ScrollView's
'implicitHeight'.

To get rid of the binding loop on the 'implicitHeight' property, check
if the TextEdit's updateSize() has already been called, and if so, don't
emit its contentSizeChanged() signal again.

In order to get rid of the binding loop on the Scrollbars' 'visible'
property, introduce two new properties: effectiveScrollBarWidth and
effectiveScrollBarHeight which represent the actual width/height of
the scrollbars. Set this property dependent on the scrollbar's policy,
visibility, and the scrollview's content size and make sure to update
whenever any of these changes. Also, follow the same pattern as above
and don't emit the signal twice if on a recursive call.

[ChangeLog][Controls][ScrollView] Added effectiveScrollBarWidth and
effectiveScrollBarHeight which hold the effective sizes of scrollbars.

Fixes: QTBUG-97974
Change-Id: I3d915eae53881d000769de1824c3908c7acd5c6b
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Doris Verria <doris.verria@qt.io>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
This commit is contained in:
Kaj Grönholm 2023-04-05 11:19:31 +03:00
parent 064aa8127c
commit bdb7bc0543
7 changed files with 157 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/quick/items/qquicktextedit.cpp
View File

@ -2707,7 +2707,12 @@ void QQuickTextEdit::updateSize()
QSizeF size(newWidth, newHeight);
if (d->contentSize != size) {
d->contentSize = size;
emit contentSizeChanged();
// Note: inResize is a bitfield so QScopedValueRollback can't be used here
const bool wasInResize = d->inResize;
d->inResize = true;
if (!wasInResize)
emit contentSizeChanged();
d->inResize = wasInResize;
updateTotalLines();
}
}

3
src/quick/items/qquicktextedit_p_p.h
View File

@ -95,7 +95,7 @@ public:
, focusOnPress(true), persistentSelection(false), requireImplicitWidth(false)
, selectByMouse(true), canPaste(false), canPasteValid(false), hAlignImplicit(true)
, textCached(true), inLayout(false), selectByKeyboard(false), selectByKeyboardSet(false)
, hadSelection(false), markdownText(false)
, hadSelection(false), markdownText(false), inResize(false)
{
}
@ -202,6 +202,7 @@ public:
bool selectByKeyboardSet:1;
bool hadSelection : 1;
bool markdownText : 1;
bool inResize : 1;
static const int largeTextSizeThreshold;
};

4
src/quickcontrols/macos/ScrollView.qml
View File

@ -14,8 +14,8 @@ T.ScrollView {
implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
contentHeight + topPadding + bottomPadding)
rightPadding: ScrollBar.vertical.visible ? ScrollBar.vertical.width : 0
bottomPadding: ScrollBar.horizontal.visible ? ScrollBar.horizontal.height : 0
rightPadding: effectiveScrollBarWidth
bottomPadding: effectiveScrollBarHeight
// Don't set __notCustomizable here, because it would require special-casing
// setFlickable's call to setContentItem.

4
src/quickcontrols/windows/ScrollView.qml
View File

@ -13,8 +13,8 @@ T.ScrollView {
implicitHeight: Math.max(implicitBackgroundHeight + topInset + bottomInset,
contentHeight + topPadding + bottomPadding)
rightPadding: ScrollBar.vertical.visible ? ScrollBar.vertical.width : 0
bottomPadding: ScrollBar.horizontal.visible ? ScrollBar.horizontal.height : 0
rightPadding: effectiveScrollBarWidth
bottomPadding: effectiveScrollBarHeight
// Don't set __notCustomizable here, because it would require special-casing
// setFlickable's call to setContentItem.

130
src/quicktemplates/qquickscrollview.cpp
View File

@ -132,10 +132,17 @@ public:
void itemImplicitWidthChanged(QQuickItem *item) override;
void updateScrollBarWidth();
void updateScrollBarHeight();
void disconnectScrollBarSignals(QQuickScrollBarAttachedPrivate *scrollBar);
bool wasTouched = false;
QQuickFlickable *flickable = nullptr;
bool flickableHasExplicitContentWidth = true;
bool flickableHasExplicitContentHeight = true;
bool isUpdatingScrollBar = false;
qreal effectiveScrollBarWidth = 0;
qreal effectiveScrollBarHeight = 0;
};
QList<QQuickItem *> QQuickScrollViewPrivate::contentChildItems() const
@ -176,6 +183,58 @@ QQuickFlickable *QQuickScrollViewPrivate::ensureFlickable(ContentItemFlag conten
return flickable;
}
void QQuickScrollViewPrivate::updateScrollBarWidth()
{
Q_Q(QQuickScrollView);
qreal oldEffectiveScrollBarWidth = effectiveScrollBarWidth;
if (auto *vBar = verticalScrollBar()) {
if (vBar->policy() == QQuickScrollBar::AlwaysOff || !vBar->isVisible())
effectiveScrollBarWidth = 0;
else
effectiveScrollBarWidth = vBar->width();
}
if (effectiveScrollBarWidth != oldEffectiveScrollBarWidth) {
if (!isUpdatingScrollBar) {
QScopedValueRollback<bool> rollback(isUpdatingScrollBar, true);
emit q->effectiveScrollBarWidthChanged();
}
}
}
void QQuickScrollViewPrivate::updateScrollBarHeight()
{
Q_Q(QQuickScrollView);
qreal oldEffectiveScrollBarHeight = effectiveScrollBarHeight;
if (auto *hBar = horizontalScrollBar()) {
if (hBar->policy() == QQuickScrollBar::AlwaysOff || !hBar->isVisible())
effectiveScrollBarHeight = 0;
else
effectiveScrollBarHeight = hBar->height();
}
if (effectiveScrollBarHeight != oldEffectiveScrollBarHeight) {
if (!isUpdatingScrollBar) {
QScopedValueRollback<bool> rollback(isUpdatingScrollBar, true);
emit q->effectiveScrollBarHeightChanged();
}
}
}
void QQuickScrollViewPrivate::disconnectScrollBarSignals(QQuickScrollBarAttachedPrivate *scrollBar)
{
if (!scrollBar)
return;
if (scrollBar->vertical) {
QObjectPrivate::disconnect(scrollBar->vertical, &QQuickScrollBar::policyChanged, this, &QQuickScrollViewPrivate::updateScrollBarWidth);
QObjectPrivate::disconnect(scrollBar->vertical, &QQuickScrollBar::visibleChanged, this, &QQuickScrollViewPrivate::updateScrollBarWidth);
}
if (scrollBar->horizontal) {
QObjectPrivate::disconnect(scrollBar->horizontal, &QQuickScrollBar::policyChanged, this, &QQuickScrollViewPrivate::updateScrollBarHeight);
QObjectPrivate::disconnect(scrollBar->horizontal, &QQuickScrollBar::visibleChanged, this, &QQuickScrollViewPrivate::updateScrollBarHeight);
}
}
bool QQuickScrollViewPrivate::setFlickable(QQuickFlickable *item, ContentItemFlag contentItemFlag)
{
Q_Q(QQuickScrollView);
@ -187,8 +246,11 @@ bool QQuickScrollViewPrivate::setFlickable(QQuickFlickable *item, ContentItemFla
if (flickable) {
flickable->removeEventFilter(q);
if (attached)
QQuickScrollBarAttachedPrivate::get(attached)->setFlickable(nullptr);
if (attached) {
auto *scrollBar = QQuickScrollBarAttachedPrivate::get(attached);
scrollBar->setFlickable(nullptr);
disconnectScrollBarSignals(scrollBar);
}
QObjectPrivate::disconnect(flickable->contentItem(), &QQuickItem::childrenChanged, this, &QQuickPanePrivate::contentChildrenChange);
QObjectPrivate::disconnect(flickable, &QQuickFlickable::contentWidthChanged, this, &QQuickScrollViewPrivate::flickableContentWidthChanged);
@ -210,8 +272,18 @@ bool QQuickScrollViewPrivate::setFlickable(QQuickFlickable *item, ContentItemFla
else
flickableContentHeightChanged();
if (attached)
QQuickScrollBarAttachedPrivate::get(attached)->setFlickable(flickable);
if (attached) {
auto *scrollBar = QQuickScrollBarAttachedPrivate::get(attached);
scrollBar->setFlickable(flickable);
if (scrollBar->vertical) {
QObjectPrivate::connect(scrollBar->vertical, &QQuickScrollBar::policyChanged, this, &QQuickScrollViewPrivate::updateScrollBarWidth);
QObjectPrivate::connect(scrollBar->vertical, &QQuickScrollBar::visibleChanged, this, &QQuickScrollViewPrivate::updateScrollBarWidth);
}
if (scrollBar->horizontal) {
QObjectPrivate::connect(scrollBar->horizontal, &QQuickScrollBar::policyChanged, this, &QQuickScrollViewPrivate::updateScrollBarHeight);
QObjectPrivate::connect(scrollBar->horizontal, &QQuickScrollBar::visibleChanged, this, &QQuickScrollViewPrivate::updateScrollBarHeight);
}
}
QObjectPrivate::connect(flickable->contentItem(), &QQuickItem::childrenChanged, this, &QQuickPanePrivate::contentChildrenChange);
QObjectPrivate::connect(flickable, &QQuickFlickable::contentWidthChanged, this, &QQuickScrollViewPrivate::flickableContentWidthChanged);
@ -418,6 +490,48 @@ QQuickScrollView::QQuickScrollView(QQuickItem *parent)
setWheelEnabled(true);
}
QQuickScrollView::~QQuickScrollView()
{
Q_D(QQuickScrollView);
QQuickScrollBarAttached *attached = qobject_cast<QQuickScrollBarAttached *>(qmlAttachedPropertiesObject<QQuickScrollBar>(this, false));
if (attached) {
auto *scrollBar = QQuickScrollBarAttachedPrivate::get(attached);
d->disconnectScrollBarSignals(scrollBar);
}
}
/*!
\qmlproperty real QtQuick.Controls::ScrollView::effectiveScrollBarWidth
\since 6.6
This property holds the effective width of the vertical scrollbar.
When the scrollbar policy is \c QQuickScrollBar::AlwaysOff or the scrollbar
is not visible, this property is \c 0.
\sa {ScrollBar::policy}
*/
qreal QQuickScrollView::effectiveScrollBarWidth()
{
Q_D(QQuickScrollView);
return d->effectiveScrollBarWidth;
}
/*!
\qmlproperty real QtQuick.Controls::ScrollView::effectiveScrollBarHeight
\since 6.6
This property holds the effective height of the horizontal scrollbar.
When the scrollbar policy is \c QQuickScrollBar::AlwaysOff or the scrollbar
is not visible, this property is \c 0.
\sa {ScrollBar::policy}
*/
qreal QQuickScrollView::effectiveScrollBarHeight()
{
Q_D(QQuickScrollView);
return d->effectiveScrollBarHeight;
}
/*!
\qmlproperty list<QtObject> QtQuick.Controls::ScrollView::contentData
\qmldefault
@ -603,10 +717,14 @@ void QQuickScrollView::contentSizeChange(const QSizeF &newSize, const QSizeF &ol
// exception is if the application has assigned a content size
// directly to the scrollview, which will then win even if the
// application has assigned something else to the flickable.
if (d->hasContentWidth || !d->flickableHasExplicitContentWidth)
if (d->hasContentWidth || !d->flickableHasExplicitContentWidth) {
d->flickable->setContentWidth(newSize.width());
if (d->hasContentHeight || !d->flickableHasExplicitContentHeight)
d->updateScrollBarWidth();
}
if (d->hasContentHeight || !d->flickableHasExplicitContentHeight) {
d->flickable->setContentHeight(newSize.height());
d->updateScrollBarHeight();
}
}
}

9
src/quicktemplates/qquickscrollview_p.h
View File

@ -27,9 +27,14 @@ class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickScrollView : public QQuickPane
Q_OBJECT
QML_NAMED_ELEMENT(ScrollView)
QML_ADDED_IN_VERSION(2, 2)
Q_PROPERTY(qreal effectiveScrollBarWidth READ effectiveScrollBarWidth NOTIFY effectiveScrollBarWidthChanged FINAL REVISION(6, 6))
Q_PROPERTY(qreal effectiveScrollBarHeight READ effectiveScrollBarHeight NOTIFY effectiveScrollBarHeightChanged FINAL REVISION(6, 6))
public:
explicit QQuickScrollView(QQuickItem *parent = nullptr);
~QQuickScrollView();
qreal effectiveScrollBarWidth();
qreal effectiveScrollBarHeight();
protected:
bool childMouseEventFilter(QQuickItem *item, QEvent *event) override;
@ -44,6 +49,10 @@ protected:
QAccessible::Role accessibleRole() const override;
#endif
Q_SIGNALS:
Q_REVISION(6, 6) void effectiveScrollBarWidthChanged();
Q_REVISION(6, 6) void effectiveScrollBarHeightChanged();
private:
Q_DISABLE_COPY(QQuickScrollView)
Q_DECLARE_PRIVATE(QQuickScrollView)

12
tests/auto/quickcontrols/controls/data/tst_scrollview.qml
View File

@ -188,6 +188,18 @@ TestCase {
horizontal.increase()
verify(horizontal.position > 0)
compare(control.contentItem.visibleArea.xPosition, horizontal.position)
vertical.policy = ScrollBar.AlwaysOn
horizontal.policy = ScrollBar.AlwaysOn
verify(control.effectiveScrollBarWidth > 0)
verify(control.effectiveScrollBarHeight > 0)
vertical.policy = ScrollBar.AlwaysOff
horizontal.policy = ScrollBar.AlwaysOff
compare(control.effectiveScrollBarWidth, 0)
compare(control.effectiveScrollBarHeight, 0)
}
function test_oneChild_data() {