Merge lp://qastaging/~zsombi/ubuntu-ui-toolkit/35-options-panel-swipe into lp://qastaging/~zsombi/ubuntu-ui-toolkit/listitem-master

I don't see icons with this code: http://pastebin.ubuntu.com/9145869/

with the code above, the list item text never becomes red, so swiping never becomes true?

66 + // for testing
67 + objectName: "ListItemPanel" + (leadingPanel ? "Leading" : "Trailing")

That might appear like you left some debugging code that should not be in the final version.

Change the comment to // Used for autopilot testing in filename.py?
or something similar.

74 + Specifies whether the panel is used to visualize leading or trailing options.
75 + */
76 + readonly property bool leadingPanel: panel.ListItemActions.status == panel.ListItemActions.Leading

I prefer readonly property bool leading.
We know we are a panel.

> 66 + // for testing
> 67 + objectName: "ListItemPanel" + (leadingPanel ? "Leading" :
> "Trailing")
>
> That might appear like you left some debugging code that should not be in the
> final version.
>
> Change the comment to // Used for autopilot testing in filename.py?
> or something similar.

It is used in module testing.

321 + , xAxisMoveThresholdGU(1.5)
322 , overshoot(UCUnits::instance().gu(2))

let's do the x-axis threshold like the overshoot, so:
xAxisMoveThreshold(UCUnits::instance().gu(1.5))

372 + if (!UCListItemActionsPrivate::isConnectedTo(leadingActions, q) && !UCListItemActionsPrivate::isConnectedTo(trailingActions, q)) {

add a linebreak

383 + * animations. The component does not assume any visuals present in the style,
384 + * and will load its content only when requested, i.e. either of the mentioned
385 + * components are visualized.

the part from i.e. is a bit unclear to me. What do you try to say there?

458 + setTugged(false);

didn't we switch from "tug" to "swipe" in a previous MR?

should we include a link to ListItemStyle in the ListItem docs?

Can we improve this text?

> The component defines the style API for the ListItem component.
> Provides the default actions visualization panel, snap animation,
> selection mode and drag handler delegates. ListItem treats the
> style differently compared to the other components, as it will
> load it only when needed, to load the components defining the
> look and feel of the different configurable elements of the component,
> and will ignore any visuals declared in the style.

Style API for the ListItem component which provides actions, select and
drag handler delegates, and snap animation via its properties.
ListItem treats the style differently compared to the other components,
as it:
- loads the style only when needed
- gets delegates and snap animation from the style properties
- ignores any other visuals defined in the style

replace "tug" by "swipe" everywhere?

Remove this completely?
I understand the example, but I think it is bad practice to show the same action on both sides, so we should not include it as a code example.

 If
568 + * it is desired to have the same action present in both leading and trailing
569 + * actions, one of the ListItemActions actions list can use the other's list. In
570 + * the following example the list item can be deleted through both leading and
571 + * trailing actions:
572 + * \qml
573 + * ListItem {
574 + * id: listItem
575 + * leadingActions: ListItemActions {
576 + * actions: [
577 + * Action {
578 + * iconName: "delete"
579 + * onTriggered: listItem.destroy()
580 + * }
581 + * ]
582 + * }
583 + * trailingActions: ListItemActions {
584 + * actions: leadingActions.actions
585 + * }
586 + * }

1478 === added file 'tests/unit/tst_performance/ListItemWithInlineOptionsList.qml'

filename options-->actions

> with the code above, the list item text never becomes red, so swiping never
> becomes true?

First, ListItem attached properties are only valid when attached to their parent, or if used in ListView, then attached to ListView, or when used in Flickables, attached to Flickables.

Second, swiping is not defined for ListItem, but for ListItemAttached. Now that property is valid only for leadin/trailing panels.

It is unfortunate that you can attach the property to any object, but this is something we cannot change. The same happens with ListView attached properties, it doesn't give you any warning/error (as it cannot) if attached to other elements but the ListView's delegate, but only works with the delegate.

> 321 + , xAxisMoveThresholdGU(1.5)
> 322 , overshoot(UCUnits::instance().gu(2))
>
> let's do the x-axis threshold like the overshoot, so:
> xAxisMoveThreshold(UCUnits::instance().gu(1.5))

No, I would not. Especially that this is not configurable. If we decide to make it configurable, we can do it later. Yet, there's no reason to watch the units change to update this property, as we need the threshold only when swiping. And if we don't configure it, we can always get the proper pixel amount by converting in place.

> 383 + * animations. The component does not assume any visuals present in
> the style,
> 384 + * and will load its content only when requested, i.e. either of the
> mentioned
> 385 + * components are visualized.
>
> the part from i.e. is a bit unclear to me. What do you try to say there?

Perhaps that si not even needed... Will remove it.

> 458 + setTugged(false);
>
> didn't we switch from "tug" to "swipe" in a previous MR?

The concept yes, but this staid... I'll change it to swipe

> should we include a link to ListItemStyle in the ListItem docs?

Yep. Added.

> Can we improve this text?
>
> > The component defines the style API for the ListItem component.
> > Provides the default actions visualization panel, snap animation,
> > selection mode and drag handler delegates. ListItem treats the
> > style differently compared to the other components, as it will
> > load it only when needed, to load the components defining the
> > look and feel of the different configurable elements of the component,
> > and will ignore any visuals declared in the style.
>
> Style API for the ListItem component which provides actions, select and
> drag handler delegates, and snap animation via its properties.
> ListItem treats the style differently compared to the other components,
> as it:
> - loads the style only when needed
> - gets delegates and snap animation from the style properties
> - ignores any other visuals defined in the style

Done.

> replace "tug" by "swipe" everywhere?

No. We agreed with design that they call it tug, so we will, at least in the doc.

> Remove this completely?
> I understand the example, but I think it is bad practice to show the same
> action on both sides, so we should not include it as a code example.
>
>
> If
> 568 + * it is desired to have the same action present in both leading and
> trailing
> 569 + * actions, one of the ListItemActions actions list can use the
> other's list. In
> 570 + * the following example the list item can be deleted through both
> leading and
> 571 + * trailing actions:
> 572 + * \qml
> 573 + * ListItem {
> 574 + * id: listItem
> 575 + * leadingActions: ListItemActions {
> 576 + * actions: [
> 577 + * Action {
> 578 + * iconName: "delete"
> 579 + * onTriggered: listItem.destroy()
> 580 + * }
> 581 + * ]
> 582 + * }
> 583 + * trailingActions: ListItemActions {
> 584 + * actions: leadingActions.actions
> 585 + * }
> 586 + * }

Why not? this is a practice some apps may apply. So why shouldn't we have it documented on how to do it? This way we can be sure we won't spend useless time by triaging these kind of bugs...

okay, thanks