Code review comment for lp:~zsombi/ubuntu-ui-toolkit/80-selection-mode

Revision history for this message
Cris Dywan (kalikiana) wrote :

> 747 + self.wait_select_single('QQuickItem', objectName='selection_panel0')

A proper error message in a ToolkitException is desirable here I think.

The "new list items" section in the gallery prints "qml: Highlighting list item" when tapping, overlooked debugging? More importantly I don't see selection mode demonstrated at all.

I've used tests/resources/listitems/ListItemTest.qml for testing a bit, unfortunately it doesn't scroll so I'm not sure if I can see everything in there. At the bottom there's a very narrow item with a switch.

The test_listitem.ListItemTestCase.qml has "onPressAndHold: listView.ViewItems.selectMode = true", I'm thinking this should be available in the gallery and manual test case as well.

Visual observation: the animation from toggling select mode flickers extremely. Is this expected?

> 973 - flick(item, centerOf(item).x, centerOf(item).y, units.gu(20), 0);
> 974 + swipe(item, centerOf(item).x, centerOf(item).y, units.gu(20), 0);

Why is this? Work-around? Something to document?

> 268 + * attached property. This property is attached to each parent item of the ListItem

I don't really understand the explanation and the example doesn't demonstrate what I think it says. There's no ListView here. The Flickable has the attached property but Column supposedly won't work. I would expect Column to be the parent and have the property. Aside from clarifying the wording I'd suggest two examples because of the special case.

review: Needs Fixing

« Back to merge proposal