5652 + \qmltype MenuItemActionValue
    5653 + \inqmlmodule Indicators 0.1
    5654 + \brief Helper class to connect dbus action state with qml components property

    5656 + Exampiles:

    5672 +Item {

    It's "MenuItemAction" here, should be "MenuAction", even? And the \brief is copied from MenuItemActionValue, should say something different?

    Exampiles ← typo.

    Could it be a QtObject instead?

    =====

    5695 + // internal
    5696 + property QtObject __actionObject: null

    5707 + Item {
    5708 + id: priv
    5709 + property var actionObject: null
    5710 +
    5711 + Connections {

    Seems like priv.actionObject was supposed to replace __actionObject? Please complete that, and use "id: d" for the private object. There should be no need for the wrapping Item, either. Please use the Connections object as the private one.

    Now I see it's used in MenuItemActionValue, so I wonder if just exposing actionObject as public API of MenuItemAction wouldn't be cleaner...

Made a public property.

    =====

    5750 + \qmltype MenuItemActionValue

    Should this maybe be "MenuActionBinding"?

Renamed

    =====

    5783 + property string property: propertyBinding.property

    Couldn't this be an alias, too?

I has some issues with this. Apparently you dont get notifications for changes to binding properties, so I needed to make them root properties in order to update the connections. I've updated the target to be a property rather than an alias for the same reason.

    =====

    5795 + Item {
    5796 + id: priv
    5797 + property alias actionObject : dbusActionState.__actionObject
    5798 +
    5799 + property var currentTarget: undefined
    5800 + property string currentSignal
    5801 +
    5802 +
    5803 + Binding {

    Could be flattened to just Binding { }.

Removed the actionObject. The currentTarget/Signal is used to disconnect old property updates in connectViewChanges. Will be removed one we fix up the on%1Changed part.

    =====

    5821 + var signal = "on%1Changed".arg(property.charAt(0).toUpperCase() + property.slice(1));

    I wonder if this could be cleaner - I'd maybe even move this to C++ to mimic Binding { } more. Please add a TODO to re-evaluate at some point.

It's a bit of a  mess. Added TODO

    =====

    5867 + id: __menuFactory

    5914 + id: __loader

    5980 + id: _progressMenu

    6047 + id: __backgroundItem

    6052 + id: __backgroundText

    6115 + id: __sectionMenu

    No need to prefix those with _.

    =====

    5995 + color: "#00000000"

    color: "transparent"

    =====

    6069 + anchors.rightMargin: __backgroundText.slidingMargin
    6070 + anchors.leftMargin: 0

    6079 + anchors.rightMargin: 0
    6080 + anchors.leftMargin: __backgroundText.slidingMargin

    These could just be static margins, no?

yep.

    =====

    6122 + anchors {
    6123 + left: parent ? parent.left : undefined
    6124 + right: parent ? parent.right : undefined
    6125 + }

    Didn't you say that no menu items have that? ;)

I may have said that :). removed.

    =====

    Do we have a use for SectionMenuItem yet?

It's used by the WifiSection. Shows the spinner while waiting for wifi access points.

    =====

    6191 + width: _sliderMenu.text ? units.gu(20) : _sliderMenu.width - units.gu(4)

    Is that desired? I.e. do we want only 20GU-wide slider in that case? Do we have a usecase?

It's used by the volume menu in the sound indicator. Possibly should be using the width of text, but we don't have access to that.
May have to speak with SDK. This control will may be replaced with the ubuntu-settings-components control.

    =====

    6202 + Binding {
    6203 + target: slider.ItemStyle.style
    6204 + property: "backgroundColor"
    6205 + value: Qt.rgba(0.5, 0.5, 0.5, 0.1)
    6206 + }
    6207 +
    6208 + Binding {
    6209 + target: slider.ItemStyle.style
    6210 + property: "backgroundOpacity"
    6211 + value: 1.0
    6212 + }

    This goes away with https://code.launchpad.net/~fboucault/ubuntu-ui-toolkit/simple_theming/+merge/171645

Removed.

    =====

    6214 + // Use this to disable the label, since there is not way to do it on the component.

    Please add FIXME, especially since:

    Slider.qml:
          \b Note: this function will be deprecated, and will be solved with particular
          delegates for the thumb.
         */
        function formatValue(v) {

Added FIXME

    =====

    More incoming...