Merge lp://qastaging/~3v1n0/ubuntu-settings-components/slots-layouts into lp://qastaging/ubuntu-settings-components
- slots-layouts
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Lukáš Tinkl |
Approved revision: | 250 |
Merged at revision: | 167 |
Proposed branch: | lp://qastaging/~3v1n0/ubuntu-settings-components/slots-layouts |
Merge into: | lp://qastaging/ubuntu-settings-components |
Diff against target: |
3594 lines (+1298/-1098) 45 files modified
debian/changelog (+7/-0) examples/MessageComponents.qml (+24/-0) examples/OtherComponents.qml (+63/-6) examples/SettingsComponents.qml (+9/-4) plugins/Ubuntu/Settings/Components/ActionTextField.qml (+10/-13) plugins/Ubuntu/Settings/Components/MessageHeader.qml (+61/-72) plugins/Ubuntu/Settings/Components/QuickReply.qml (+0/-165) plugins/Ubuntu/Settings/Components/qmldir (+0/-1) plugins/Ubuntu/Settings/Menus/AccessPointMenu.qml (+30/-66) plugins/Ubuntu/Settings/Menus/BaseLayoutMenu.qml (+52/-0) plugins/Ubuntu/Settings/Menus/BaseMenu.qml (+61/-58) plugins/Ubuntu/Settings/Menus/ButtonMenu.qml (+5/-5) plugins/Ubuntu/Settings/Menus/CalendarMenu.qml (+28/-34) plugins/Ubuntu/Settings/Menus/CheckableMenu.qml (+15/-62) plugins/Ubuntu/Settings/Menus/EventMenu.qml (+9/-45) plugins/Ubuntu/Settings/Menus/GroupedMessageMenu.qml (+4/-5) plugins/Ubuntu/Settings/Menus/MediaPlayerMenu.qml (+37/-91) plugins/Ubuntu/Settings/Menus/ModemInfoItem.qml (+82/-82) plugins/Ubuntu/Settings/Menus/PlaybackButton.qml (+44/-0) plugins/Ubuntu/Settings/Menus/PlaybackItemMenu.qml (+25/-65) plugins/Ubuntu/Settings/Menus/ProgressBarMenu.qml (+12/-12) plugins/Ubuntu/Settings/Menus/ProgressValueMenu.qml (+5/-4) plugins/Ubuntu/Settings/Menus/SectionMenu.qml (+8/-12) plugins/Ubuntu/Settings/Menus/SeparatorMenu.qml (+5/-4) plugins/Ubuntu/Settings/Menus/SimpleMessageMenu.qml (+30/-36) plugins/Ubuntu/Settings/Menus/SliderMenu.qml (+43/-54) plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml (+44/-41) plugins/Ubuntu/Settings/Menus/StandardMenu.qml (+40/-0) plugins/Ubuntu/Settings/Menus/SwitchMenu.qml (+12/-42) plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml (+8/-7) plugins/Ubuntu/Settings/Menus/TimeZoneMenu.qml (+4/-8) plugins/Ubuntu/Settings/Menus/TransferMenu.qml (+25/-31) plugins/Ubuntu/Settings/Menus/UserSessionMenu.qml (+10/-15) plugins/Ubuntu/Settings/Menus/qmldir (+2/-0) tests/qmltests/CMakeLists.txt (+3/-0) tests/qmltests/Components/tst_ActionTextField.qml (+103/-0) tests/qmltests/Components/tst_ServerPropertySynchroniser.qml (+2/-2) tests/qmltests/Menus/tst_BaseMenu.qml (+182/-0) tests/qmltests/Menus/tst_CheckableMenu.qml (+35/-20) tests/qmltests/Menus/tst_GroupedMessageMenu.qml (+5/-0) tests/qmltests/Menus/tst_SimpleMessageMenu.qml (+5/-0) tests/qmltests/Menus/tst_SnapDecisionMenu.qml (+9/-1) tests/qmltests/Menus/tst_StandardMenu.qml (+81/-0) tests/qmltests/Menus/tst_SwitchMenu.qml (+53/-35) tests/qmltests/Menus/tst_TextMessageMenu.qml (+6/-0) |
To merge this branch: | bzr merge lp://qastaging/~3v1n0/ubuntu-settings-components/slots-layouts |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Approve | |
Michael Zanetti (community) | Approve | ||
Lukáš Tinkl (community) | Approve | ||
Nick Dedekind (community) | Needs Information | ||
Michael Terry | Approve | ||
Andrea Bernabei | Pending | ||
Review via email: mp+305721@code.qastaging.launchpad.net |
Commit message
Menus: rewrite components using ListItemLayout's and SlotsLayout's
Description of the change
* Are there any related MPs required for this MP to build/function as expected? Please list.
No
* Did you perform an exploratory manual test run of your code change and any related functionality?
Sure, running these with both the examples and untiy8
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
* If you changed the UI, has there been a design review?
Using SDK compoments that have already been ACK'ed by design
Michael Terry (mterry) wrote : | # |
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Hi Martin, thanks for the initial review
> I started poking at this, I know you're not marked for review yet. Mainly I
> just compared old and new examples/ and had a few comments:
No, it's fine... I've still to refactor the MessageHeader, but a part from that all the components should now have almost the final shape.
> - I assume the lighter text color and indented menu content is from the
> switched SDK element, not something explicitly done?
Exactly... And that of course won't affect unity shell, since these elements are differently themed.
> - ButtonMenu seems to have swapped its button location with
> "SlotsLayout.
Yeah, good catch... I probably pasted there something wrong.
I've fixed it now.
> - I know it's a little more consistent with behavior elsewhere, but having to
> swipe-right-
> swipe-right on the Messaging Menu when you have a couple messages to clear
> (and you don't want to clear ALL the messages with the "clear all" button).
> Is design aware of that potential change? Actually, how involved is design in
> this MP in general? I know most of it makes 100% sense (using proper new SDK
> elements is always good), but there will be some inevitable UI differences.
Mh, I agree that's annoying for notifications... This is the design though.
I think we can discuss with them this further to have a special case in that scenario.
> - Why is the width of 'qmlscene examples/
> on trunk but not on this MP?
Ups Sorry, because for testing reasons I was launching only the OtherComponents file so I set some sizes on it... Reverted :-)
> I love that every gosh dang menu item no longer has a divider by default.
Agree, probably this something that could change again, but in general there will be more groups.
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
s/Martin/Michael/
I was about to write mterry, and I mixed the two in Martin it seems :-D.
Sorry :)
Michael Terry (mterry) wrote : | # |
>> - Why is the width of 'qmlscene examples/
>> on trunk but not on this MP?
>
> Ups Sorry, because for testing reasons I was launching only the OtherComponents
> file so I set some sizes on it... Reverted :-)
No no, I liked the size being set on OtherComponents. My question was about SettingsComponents. In trunk, SettingsComponents comes up and the list elements have the correct window width. But in this MP, SettingsComponents comes up and list elements have a width bigger than the window until you resize the window. Is that indicative of a bug?
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> No no, I liked the size being set on OtherComponents. My question was about
> SettingsComponents. In trunk, SettingsComponents comes up and the list
> elements have the correct window width. But in this MP, SettingsComponents
> comes up and list elements have a width bigger than the window until you
> resize the window. Is that indicative of a bug?
I don't think it's a problem in the components themselves, but maybe more related to qmlscene (I was discussing about a similar thing with faenil too)... In fact if I launch that with qml it works as expected. I've added a workaround to make it work in both cases though.
Anyway, I've finished all the components now (I've completed this by mostly rewriting the Message menu items) so the MP is now fully open for business.
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:216
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:217
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:219
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:220
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:222
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:223
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:224
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:225
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:228
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:230
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:236
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Michael Terry (mterry) wrote : | # |
So glancing over the code changes, they seem fine.
I loaded this up on a test phone. Love the new look in general, except for one big annoyance: the forced padding on the left.
Consider the Rotation menu. It has only one entry, but a bunch of padding on the left. Looks odd.
Then look at the Network menu. The cellular status label is left-aligned, but most of the other control widget labels have left-padding instead.
And on the Sound menu, "Volume" is left-aligned and looks fine. But "Silent Mode" isn't and looks odd right next to it.
Is there a fix to make those look more natural? The easiest would be to drop back to no-left padding like we had before, but maybe there are better solutions.
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> So glancing over the code changes, they seem fine.
>
> I loaded this up on a test phone. Love the new look in general, except for
> one big annoyance: the forced padding on the left.
>
> Consider the Rotation menu. It has only one entry, but a bunch of padding on
> the left. Looks odd.
>
> Then look at the Network menu. The cellular status label is left-aligned, but
> most of the other control widget labels have left-padding instead.
>
> And on the Sound menu, "Volume" is left-aligned and looks fine. But "Silent
> Mode" isn't and looks odd right next to it.
>
> Is there a fix to make those look more natural? The easiest would be to drop
> back to no-left padding like we had before, but maybe there are better
> solutions.
So... I forgot to hide the icon if the source wasn't set, so I've fixed that... Now it should look as it used to be.
Thanks for noticing it!
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:237
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Michael Terry (mterry) wrote : | # |
Nice, much better.
Why is the text on "Charge level" in Battery darker than other text?
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> Nice, much better.
>
> Why is the text on "Charge level" in Battery darker than other text?
Well, this is what we get from the model:
label: Charge level
sensitive: false
isSeparator: false
icon:
type: com.canonical.
ext:
action: indicator.
actionState: 100
isCheck: false
isRadio: false
isToggled: false
Thus the "sensitive: false" thing causes the "enabled" property to be set to false. This is done in MenuItemFactory in unity8.
In that case, as you can see in BaseLayoutMenu the title's opacity is set to 0.5.
I think that's correct in normal state...
However I can easily fix this at U8 level by hacking it with: http://
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:238
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Michael Terry (mterry) wrote : | # |
Nice, thanks. Looks good to me now.
Nick Dedekind (nick-dedekind) wrote : | # |
There are a bunch of print statements in the code. Presumably these were for debugging only?
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> There are a bunch of print statements in the code. Presumably these were for
> debugging only?
Yes, sure... They're just in the example client
Lukáš Tinkl (lukas-kde) wrote : | # |
Activating access points doesn't work with this branch (neither in indicators, nor from System Settings)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:250
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Lukáš Tinkl (lukas-kde) wrote : | # |
I think this is due to the fact the AccessPointMenu is a checkable menu, eventhough it doesn't contain a (visual) checkbox
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
I'm not sure I got what you mean, it seems fine here: http://
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
While tapping on it I'm properly getting the expected behavior http://
Lukáš Tinkl (lukas-kde) wrote : | # |
Indeed, the problem seems to be somewhere lower in the stack
- 251. By Marco Trevisan (Treviño)
-
MessageHeader: align the app icon to the summary text if there's a time text
- 252. By Marco Trevisan (Treviño)
-
MessageHeader: no need to add binding for text
- 253. By Marco Trevisan (Treviño)
-
MessageHeader: refer to title, as summary might be empty
- 254. By Marco Trevisan (Treviño)
-
MessageHeader: use an AbstractButton for icon click
- 255. By Marco Trevisan (Treviño)
-
MessageHeader: define icon y position based on time... being the more relevant element for it
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:255
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 256. By Marco Trevisan (Treviño)
-
BaseMenu, SimpleMessageMenu: enable clipping if there are trailing or leading actions
Michael Zanetti (mzanetti) wrote : | # |
tested the latest commits. all good still
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:255
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
I started poking at this, I know you're not marked for review yet. Mainly I just compared old and new examples/ and had a few comments:
- I assume the lighter text color and indented menu content is from the switched SDK element, not something explicitly done?
- ButtonMenu seems to have swapped its button location with "SlotsLayout. position: SlotsLayout. Leading" instead of trailing.
- I know it's a little more consistent with behavior elsewhere, but having to swipe-right- then-press- the-delete- button is a bit more annoying than merely swipe-right on the Messaging Menu when you have a couple messages to clear (and you don't want to clear ALL the messages with the "clear all" button). Is design aware of that potential change? Actually, how involved is design in this MP in general? I know most of it makes 100% sense (using proper new SDK elements is always good), but there will be some inevitable UI differences.
- Why is the width of 'qmlscene examples/ SettingsCompone nts.qml' set correctly on trunk but not on this MP?
I love that every gosh dang menu item no longer has a divider by default.