Header should only change flickable content margins when its size is not null and it's visible property is true

Bug #1560458 reported by Andrea Bernabei
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical System Image
Fix Released
Medium
Zoltan Balogh
ubuntu-ui-toolkit (Ubuntu)
Fix Released
Medium
Tim Peeters
ubuntu-ui-toolkit (Ubuntu RTM)
Fix Released
Medium
Tim Peeters

Bug Description

r1886

Header should not update (or it should stop updating) Flickable content margins in case either one (or both) of its sides has null size or it's !visible.
This is to be consistent with how QtQuick layout components already behave -> they ignore children whose size is null or that are !visible.

NOTE: opacity should not affect this logic. If opacity is 0, the items should NOT be ignored (this allows opacity animations).

TestCase: you can verify that flickable.topMargin is equal to editHeader's height even when the header it's not visible and standardHeader is the visible one instead.

            MainView {
                id: mainView_movingHeaderTest
                width: units.gu(50)
                height: units.gu(80)
                clip: true

                property alias page: pageItem
                property alias standardHeader: standardHeaderItem
                property alias editHeader: editHeaderItem

                Page {
                    id: pageItem
                    header: standardHeaderItem

                    Flickable {
                        id: flickable_movingHeaderTest
                        anchors.fill: parent
                        //just make sure the scrollbar is scrollable
                        contentHeight: mainView_movingHeaderTest.height * 2
                        contentWidth: mainView_movingHeaderTest.width * 2
                        Label {
                            text: "Use the icons in the header."
                            visible: standardHeaderItem.visible
                        }
                    }
                    Scrollbar {
                        id: scrollbar_movingHeaderTest
                        flickableItem: flickable_movingHeaderTest
                    }

                    PageHeader {
                        id: standardHeaderItem
                        visible: pageItem.header === standardHeaderItem
                        title: "Default title"
                        flickable: flickable_movingHeaderTest
                        trailingActionBar.actions: [
                            Action {
                                iconName: "edit"
                                text: "Edit"
                                onTriggered: pageItem.header = editHeaderItem
                            }
                        ]
                    }
                    PageHeader {
                        id: editHeaderItem
                        visible: pageItem.header === editHeaderItem
                        flickable: flickable_movingHeaderTest
                        property Component delegate: Component {
                            AbstractButton {
                                id: button
                                action: modelData
                                width: label.width + units.gu(4)
                                height: parent.height
                                Rectangle {
                                    color: UbuntuColors.slate
                                    opacity: 0.1
                                    anchors.fill: parent
                                    visible: button.pressed
                                }
                                Label {
                                    anchors.centerIn: parent
                                    id: label
                                    text: action.text
                                    font.weight: text === "Confirm"
                                                 ? Font.Normal
                                                 : Font.Light
                                }
                            }
                        }
                        leadingActionBar {
                            anchors.leftMargin: 0
                            actions: Action {
                                text: "Cancel"
                                iconName: "close"
                                onTriggered: pageItem.header = standardHeaderItem
                            }
                            delegate: editHeaderItem.delegate
                        }
                        trailingActionBar {
                            anchors.rightMargin: 0
                            actions: Action {
                                text: "Confirm"
                                iconName: "tick"
                                onTriggered: pageItem.header = standardHeaderItem
                            }
                            delegate: editHeaderItem.delegate
                        }
                        extension: Toolbar {
                            anchors {
                                left: parent.left
                                right: parent.right
                                bottom: parent.bottom
                            }
                            trailingActionBar.actions: [
                                Action { iconName: "bookmark-new" },
                                Action { iconName: "add" },
                                Action { iconName: "edit-select-all" },
                                Action { iconName: "edit-copy" },
                                Action { iconName: "select" }
                            ]
                            leadingActionBar.actions: Action {
                                iconName: "delete"
                                text: "delete"
                                onTriggered: print("Delete action triggered")
                            }
                        }
                    }
                }
            }

Related branches

Andrea Bernabei (faenil)
Changed in ubuntu-ui-toolkit (Ubuntu):
assignee: nobody → Tim Peeters (tpeeters)
description: updated
description: updated
Tim Peeters (tpeeters)
Changed in ubuntu-ui-toolkit (Ubuntu):
importance: Undecided → Medium
status: New → Confirmed
Tim Peeters (tpeeters)
Changed in ubuntu-ui-toolkit (Ubuntu):
status: Confirmed → In Progress
Revision history for this message
Andrea Bernabei (faenil) wrote :

Reference, from QtQuick Row documentation:

"If an item within a Row is not visible, or if it has a width or height of 0, the item will not be laid out and it will not be visible within the row. Also, since a Row automatically positions its children horizontally, a child item within a Row should not set its x position or horizontally anchor itself using the left, right, anchors.horizontalCenter, fill or centerIn anchors. If you need to perform these actions, consider positioning the items without the use of a Row."

Revision history for this message
Andrea Bernabei (faenil) wrote :

The same rules are followed by RowLayout, ColumnLayout and all the other Layout elements.

See https://github.com/qtproject/qtdeclarative/blob/5.7/src/imports/layouts/qquicklayout.cpp#L735

In the code linked above, shouldIgnoreItem() is a function that returns true if the item is not visible or its effective size is 0.

NOTE: shouldIgnoreItem() ignores the item if both width AND height are 0.

Changed in canonical-devices-system-image:
milestone: none → 11
Changed in canonical-devices-system-image:
status: New → In Progress
assignee: nobody → Zoltan Balogh (bzoltan)
importance: Undecided → Medium
Changed in ubuntu-ui-toolkit (Ubuntu RTM):
importance: Undecided → Medium
Changed in ubuntu-ui-toolkit (Ubuntu):
status: In Progress → Fix Committed
Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: New → Fix Committed
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

This bug was fixed in the package ubuntu-ui-toolkit 1.3.1938+15.04.20160412 in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/stable-phone-overlay

---------------

ubuntu-ui-toolkit (1.3.1938+15.04.20160412) vivid; urgency=medium

  [ Tim Peeters ]
  * Hide the PageStack back button when depth == 1. Fixes LP: #1565811
  * Add header subtitle. Fixes LP: #1399289
  * Reveal the header in gallery when changing the layout to two columns.
    Fixes LP: #1556860
  * Set correct colors for disabled actions in the header.
    Fixes LP: #1393485
  * Disable tst_datepicker.qml to unblock the staging.Fixes LP: #1567840
  * Prevent invisible header from setting the flickable topMargin.
    Fixes LP: #1560419, LP: #1560458, LP: #1566231.

  [ Zsombor Egri ]
  * Move MouseTouchAdaptor into UbuntuToolkit library. Fix adaptor code for
    Xenial. Fixes LP: #1561436

  [ Christian Dywan ]
  * Don't use a separate argument to distinguish touch events. If it's touch,
    it has our overloaded methods. Fixes LP: #1530802
  * Use export_qml_dir.sh in qmlapicheck and runtest Also add Usage to runtest.
    Fixes LP: #1567286.
  * Summary style error results with sections. Fixes LP: #1568804

  [ Andrea Bernabei ]
  * Mouse filter: check if mouse is inside the area on mouse moves and
    setHovered accordingly. Fixes LP: #1566378.

  [ Timo Jyrinki ]
  * Add latest changes for GLES.
  * Add back Provides: qtdeclarative5-ubuntu-ui-toolkit-plugin to resolve
    upgrade issue. Fixes LP: #1568817
  * Fix wrapper script auto-generated by Qt that incorrectly tries to execute
    itself. Fixes LP: #1560000
  * bileto_convert_to_gles: sort and add
    qml-module-ubuntu-performancemetrics-gles.install. Fixes LP: #1569217

  * Add additional Breaks as requested by archive admin.
  * Add latest changes for GLES.

  [ Zolán Balogh ]
  * Fix the UITK test plan script.

  [ CI Train Bot ]
  * Resync trunk. added: examples/ubuntu-ui-toolkit-gallery/po/nb.po

  [ Robert Park ]
  * Inline GLES packaging. added: debian/bileto_convert_to_gles
    debian/control.gles debian/rules.gles

 -- Zoltan Balogh <email address hidden> Tue, 12 Apr 2016 11:12:44 +0000

Changed in ubuntu-ui-toolkit (Ubuntu RTM):
status: Fix Committed → Fix Released
Changed in canonical-devices-system-image:
status: In Progress → Fix Committed
Tim Peeters (tpeeters)
Changed in ubuntu-ui-toolkit (Ubuntu RTM):
assignee: nobody → Tim Peeters (tpeeters)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-ui-toolkit - 1.3.1984+16.10.20160527.2

---------------
ubuntu-ui-toolkit (1.3.1984+16.10.20160527.2) yakkety; urgency=medium

  [ Christian Dywan ]
  * Slimmer frame for TextFields: 0.5dp. Fixes LP: #1578190.

  [ Albert Astals Cid ]
  * Add override
    The override specifier (since C++11) specifies that a virtual function
    overrides another virtual function. In a member function declaration or
    definition, override ensures that the function is virtual and is overriding
    a virtual function from the base class.

  [ Tim Peeters ]
  * Fix reference error in PullToRefreshStyle. Fixes LP: #1582843
  * Mark Tab, Tabs, TabBar, PageHeadConfiguration, PageHeadSections,
    PageHeadState, ToolbarButton, ToolbarItems as deprecated in the
    documentation. Fixes LP: #1566735, LP: #1566741.

  [ CI Train Bot ]
  * Resync trunk.

 -- Zoltan Balogh <email address hidden> Fri, 27 May 2016 07:08:44 +0000

Changed in ubuntu-ui-toolkit (Ubuntu):
status: Fix Committed → Fix Released
Changed in canonical-devices-system-image:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.