Code review comment for lp:~tpeeters/ubuntu-ui-toolkit/cppHeader

Revision history for this message
Tim Peeters (tpeeters) wrote :

> Do you expect to be able to override the Header properties? if not (especially
> exposed and flickable, but I think all) then please make them FINAL!!!!
>
> Also, see 2 comments inline. ListItem.header tests woudl be welcome!!!

I made the properties FINAL in r1676.

About the ListItem.header I am not sure what to test. You can have code like this:
Item {
  Header {
    flickable: listView
  }
  ListView {
    id: listView
    anchors.fill: parent
    model: 50
  }
}

here the Header works exactly like with a Flickable, so no additional tests are needed.

Alternatively you can have code like this:

ListView {
    model: 50
    headerPositioning: ListView.PullBackHeader
    header: Component {
        Header { }
    }
}

here, (since you don't set the flickable of the header), the Header does not manipulate its y-value so it is positioned like any Item. What would I have to test there?

The two cases give similar results, but there is a difference when scrolling, and then releasing while the header is partially exposed: in the first case it will either show or hide completely (depending on how much it was exposed), it the second case it will just stay where it is.

« Back to merge proposal