Unity 8

Merge lp:~unity-api-team/unity8/modeminfo into lp:unity8

  1. modeminfo
  2. Merge into trunk
Proposed by Antti Kaijanmäki
Status: Merged
Approved by: Nick Dedekind
Approved revision: 986
Merged at revision: 1115
Proposed branch: lp:~unity-api-team/unity8/modeminfo
Merge into: lp:unity8
Diff against target: 316 lines (+272/-0)
4 files modified
qml/Panel/Indicators/MenuItemFactory.qml (+74/-0)
qml/Panel/Indicators/ModemInfoItem.qml (+111/-0)
qml/Panel/Indicators/RoamingIndication.qml (+40/-0)
tests/qmltests/Panel/Indicators/tst_MenuItemFactory.qml (+47/-0)
To merge this branch: bzr merge lp:~unity-api-team/unity8/modeminfo
Reviewer Review Type Date Requested Status
Nick Dedekind (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Albert Astals Cid (community) Abstain
Michał Sawicz Needs Information
Matthew Paul Thomas (community) design Disapprove
Review via email: mp+225159@code.launchpad.net

Commit message

Indicators: Adds new ModemInfoItem to be used with indicator-network

Description of the change

New indicator item that displays modem information and allows unlocking of a cellular modems.

Example of the different variations:
http://imgur.com/W52T6nV

Actual result when combined with the indicator:
http://imgur.com/XK0fEWt

 * Are there any related MPs required for this MP to build/function as expected? Please list.

This MP is a prerequisite for:
https://code.launchpad.net/~unity-api-team/indicator-network/modeminfo/+merge/225160

 * Did you perform an exploratory manual test run of your code change and any related functionality?

Yes.

 * Did you make sure that your branch does not contain spurious tags?

Yes.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

Didn't touch it.

 * If you changed the UI, has there been a design review?

We asked for clarification for whether or not to have these modem items and we got a new design for the items from Esti Landa and Olga Kemmet. The design is available here (find the page that says "Network"):
https://docs.google.com/a/canonical.com/document/d/1KC40suUs13gajIlE1Ms6PJYoIPx_AjlF4ae6bvtNjNc/edit
(or here: http://imgur.com/a/g7LVB#yBWGgPz)

Packages ready for testing in silo 1

To post a comment you must log in.
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

The menu items go in lp:ubuntu-settings-components.
Only the factory creator (plus testing creation) goes into unity8.

review: Needs Fixing
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

To clarify, move ModemInfoItem.qml into lp:ubuntu-settings-components/Ubuntu/Settings/Menus

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

and RoamingIndication.qml into lp:ubuntu-settings-components/Ubuntu/Settings/Components

review: Needs Fixing
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

MP checklist:
https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8

please add required branch from indicator-network.

Item visual design doesn't match:
https://docs.google.com/a/canonical.com/document/d/1OyHUg_uUfmhDNa-9UrMc1tZ_eH_99PEU_V2l1YFA1UY/edit#
Don't know which one needs updating.

1) Status icons on left (no roaming icon on right, perhaps should go left).
   - should probably use an "x-canonical-modem-status-icons" action which has a "a(s)" variant rather than an action per icon.
2) Text should be right aligned and be in format (CARRIER - NUMBER).
3) There apparently shouldn't be SIM unlocking in indicators.
4) Is the text under roaming icon needed?

66 + shell.hideIndicatorMenu(UbuntuAnimation.BriskDuration);

While this is in other items at the moment, we're no longer going to be hiding the indicators on activations. Please remove.

45 + property var simIdentifierLabelAction: QMenuModel.UnityMenuAction {

Why is this text an action? Will it ever change?
Should this not be the label of the menu item. actions. I realise this is a backend related comment.
Only relavant if the visual design is correct.

review: Needs Fixing
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> The menu items go in lp:ubuntu-settings-components.
> Only the factory creator (plus testing creation) goes into unity8.

I'm sorry, but I don't see to point of moving an item that is entirely specific to indicator-network to ubuntu-settings-components. This item will not be used in the system-settings.

Could we not just have it in the unity8 tree?

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Apparently the current design doc may be a bit off. Not sure about the whole sim locking business.
So we can run with what you have for now for code review.

Please add a designer for reviewing this. Check with JohnLea who is resonsible for the design. Think it might be Olga.

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> MP checklist:
> https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8
>
> please add required branch from indicator-network.

Part of the MP description.

> Item visual design doesn't match:
> https://docs.google.com/a/canonical.com/document/d/1OyHUg_uUfmhDNa-
> 9UrMc1tZ_eH_99PEU_V2l1YFA1UY/edit#
> Don't know which one needs updating.

That Google document is not the working visual design for indicator-network. The tentative one is here https://wiki.ubuntu.com/Networking, but does not cover these use cases.

The best we can do ATM is to try to do sensible choices between the two designs.

> 1) Status icons on left (no roaming icon on right, perhaps should go left).
> - should probably use an "x-canonical-modem-status-icons" action which has
> a "a(s)" variant rather than an action per icon.

Roaming icon needs an explanatory text to go with it, so just having a list of icons is not going to cut it.

> 2) Text should be right aligned and be in format (CARRIER - NUMBER).

The gdoc visual design is not complete and should not be considered obligatory.

> 3) There apparently shouldn't be SIM unlocking in indicators.

indicator-network is the only place where the user can manually unlock the SIM.

> 4) Is the text under roaming icon needed?

Yes, as the roaming icon is not clear enough on it's own.

> 66 +
> shell.hideIndicatorMenu(UbuntuAnimation.BriskDuration);
>
> While this is in other items at the moment, we're no longer going to be hiding
> the indicators on activations. Please remove.

When was this decided? How do the indicators close then if you select "Wi-Fi settings" or any other item inside the indicators that open applications?

> 45 + property var simIdentifierLabelAction:
> QMenuModel.UnityMenuAction {
>
> Why is this text an action? Will it ever change?
> Should this not be the label of the menu item. actions. I realise this is a
> backend related comment.

Nothing specifies that the sim identifier could not be changed. Having it changeable gives flexibility on the backend side.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> > MP checklist:
> > https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8
> >
> > please add required branch from indicator-network.
>
> Part of the MP description.

"The indicator-network spec does not cover this item at all, but we know we need it. Having this item was agreed in Malta."

All new UI needs a design review. Especially if it's not covered in a document.
Plus, it's in the gdoc.

>
>
> > Item visual design doesn't match:
> > https://docs.google.com/a/canonical.com/document/d/1OyHUg_uUfmhDNa-
> > 9UrMc1tZ_eH_99PEU_V2l1YFA1UY/edit#
> > Don't know which one needs updating.
>
> That Google document is not the working visual design for indicator-network.
> The tentative one is here https://wiki.ubuntu.com/Networking, but does not
> cover these use cases.

Actually, the tentative one is the google doc (all the new items which may not exist in wiki yet). We use designs from designers first, even if they're provisional, and then our own ideas (reviewed by designers) if there are none. Otherwise you/me will end up having to redo the whole thing later.
I've added design review request.
Too many designers coming and going...

>
> The best we can do ATM is to try to do sensible choices between the two
> designs.
>
>
> > 1) Status icons on left (no roaming icon on right, perhaps should go left).
> > - should probably use an "x-canonical-modem-status-icons" action which
> has
> > a "a(s)" variant rather than an action per icon.
>
> Roaming icon needs an explanatory text to go with it, so just having a list of
> icons is not going to cut it.
>
>
> > 2) Text should be right aligned and be in format (CARRIER - NUMBER).
>
> The gdoc visual design is not complete and should not be considered
> obligatory.
>
>
> > 3) There apparently shouldn't be SIM unlocking in indicators.
>
> indicator-network is the only place where the user can manually unlock the
> SIM.
>
>
> > 4) Is the text under roaming icon needed?
>
> Yes, as the roaming icon is not clear enough on it's own.

If the icon is not clear enough then the icon should (possibly to have text). Raise a bug on ubuntu-themes. No other icons have labels associated to attempt to describe them.

>
>
> > 66 +
> > shell.hideIndicatorMenu(UbuntuAnimation.BriskDuration);
> >
> > While this is in other items at the moment, we're no longer going to be
> hiding
> > the indicators on activations. Please remove.
>
> When was this decided? How do the indicators close then if you select "Wi-Fi
> settings" or any other item inside the indicators that open applications?

It was decided a while back. The indicators are dismissed via context changes. App focus, stage swiping, etc.
The branch removing the other hiding is in progress, but I don't want to do multiple iterations.

>
>
> > 45 + property var simIdentifierLabelAction:
> > QMenuModel.UnityMenuAction {
> >
> > Why is this text an action? Will it ever change?
> > Should this not be the label of the menu item. actions. I realise this is a
> > backend related comment.
>
> Nothing specifies that the sim identifier could not be changed. Having it
> changeable gives flexibility on the backend side.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:981
http://jenkins.qa.ubuntu.com/job/unity8-ci/3294/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1455
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/298
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/388
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/388
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/388/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/388
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1744
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2421
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2421/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9149

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3294/rebuild

review: Approve (continuous-integration)
Revision history for this message
Matthew Paul Thomas (mpt) wrote :

Arg, what a mess.

After I had designed all the indicators for PC and phone, and made some progress in merging the designs across form factors, John asked Shruti to produce a design document for all indicators, just for phone, in a matter of weeks. The new document was, quite understandably, not nearly as detailed as the originals. For example, while the original design briefly covers SIM unlocking <https://wiki.ubuntu.com/Networking#Structure>, Shruti's does not mention it. <http://goo.gl/ms0SQX> (It is also, unfortunately, private to Canonical.)

Shruti then, also quite understandably, left the Ubuntu project. So I am unable to ask her why her abandoned design added SIM items in the menu at all. I do not think they are necessary, and they appear to be inconsistent with almost every item in every other indicator, which does something when you tap it. If these items would do something when you tap them, I have no idea what.

In Malta, Antti went with me through a list of things that needed fixing with cellular and Wi-Fi UI. According to my notes, SIM unlocking was not one of them: Antti's "was agreed in Malta" is exceeded in vagueness only by Nick's "It was decided a while back". I'd be interested to know who was involved in both cases!

The usual case for SIM unlocking will be a prompt when you turn on the phone; the indicator item is only a backstop for if you cancel that prompt then want to unlock later. So I don't think the SIM unlocking case is a good reason to have SIM items in the menu at all.

review: Disapprove (design)
Revision history for this message
Matthew Paul Thomas (mpt) wrote :

...To be precise, I don't think the SIM unlocking case is a good reason to have SIM items in the menu *all the time*. It's enough just to have an "Unlock SIM..." item when, and only when, a SIM is locked.

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> Arg, what a mess.

Agreed. But let's try to remedy :)

> So I am
> unable to ask her why her abandoned design added SIM items in the menu at all.
> I do not think they are necessary, and they appear to be inconsistent with
> almost every item in every other indicator, which does something when you tap
> it. If these items would do something when you tap them, I have no idea what.

You can't tap them. They are informative items giving description of the individual modem/sim states. These items would remedy at least and work for multiple SIMs also:
https://bugs.launchpad.net/indicator-network/+bug/1294256
https://bugs.launchpad.net/ubuntu/+source/indicator-network/+bug/1329204

> In Malta, Antti went with me through a list of things that needed fixing with
> cellular and Wi-Fi UI. According to my notes, SIM unlocking was not one of
> them: Antti's "was agreed in Malta" is exceeded in vagueness only by Nick's
> "It was decided a while back". I'd be interested to know who was involved in
> both cases!

Sorry about that. I agree I was vague. I thought we had a mutual understanding that we need additional information item inside indicator-network to show additional information that will not fit or is not suitable to be placed to the panel.

We need to schedule a meeting to discuss this.

> The usual case for SIM unlocking will be a prompt when you turn on the phone;
> the indicator item is only a backstop for if you cancel that prompt then want
> to unlock later. So I don't think the SIM unlocking case is a good reason to
> have SIM items in the menu at all.

As I said these items contain also other valuable information. With these items you can answer to questions like:

    "Which of my SIMs are roaming?",
    "From which operator is the phone getting service from?",
    "Which SIM is providing the mobile broadband connection currently?"

These are all something that IMO we can't express clearly on the panel alone and thus having the informational item inside the indicator is necessary and justified.

Revision history for this message
Matthew Paul Thomas (mpt) wrote :

So Antti and I discussed this, and here is the summary:

> https://bugs.launchpad.net/indicator-network/+bug/1294256

Already fixed.

> https://bugs.launchpad.net/ubuntu/+source/indicator-network/+bug/1329204

I'll follow up with the reporter.

> "Which of my SIMs are roaming?",
> "From which operator is the phone getting service from?",
> "Which SIM is providing the mobile broadband connection currently?"

All of these should be answerable by going to the "Cellular" screen in System Settings, where you can see which carrier is associated with which SIM, and which SIM is which color in the status bar, including the color of the roaming icon.

It's possible that carriers might require us to show the carrier name in the status bar in future, but they haven't yet (unless bug 1329204 originates with them), and the interface is simpler if we don't.

Revision history for this message
Albert Astals Cid (aacid) wrote :

Conflict adding files to plugins/Unity/Indicators/qml. Created directory.
Conflict because plugins/Unity/Indicators/qml is not versioned, but has versioned children. Versioned directory.
Text conflict in plugins/Unity/Indicators/qmldir
3 conflicts encountered.

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote :

What's the status of this?

review: Needs Information
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

WIP - updating the branch today.

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> Conflict adding files to plugins/Unity/Indicators/qml. Created directory.
> Conflict because plugins/Unity/Indicators/qml is not versioned, but has
> versioned children. Versioned directory.
> Text conflict in plugins/Unity/Indicators/qmldir
> 3 conflicts encountered.

Fixed.

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

Updated the MP description with new images and information about the design.

lp:~unity-api-team/unity8/modeminfo updated
985. By Antti Kaijanmäki

merge trunk.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:984
http://jenkins.qa.ubuntu.com/job/unity8-ci/3690/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2786
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/692
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/784
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/784
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/784/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/784
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2877
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4029
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4029/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10741

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3690/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:985
http://jenkins.qa.ubuntu.com/job/unity8-ci/3692/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2794
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/694
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/786
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/786
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/786/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/786
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2885
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4037
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4037/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10751

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3692/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Code does merge now

review: Abstain
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Code looks OK. Waiting for silo for design/testing.
Only comment is that top/bottom margin of 2gu is quite large. Takes quite a bit of vertical space. Consider 1gu top/bottom as most indicator items use this.

review: Approve
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Sorry, changed my mind :)

review: Needs Fixing
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

and more!

review: Needs Fixing
lp:~unity-api-team/unity8/modeminfo updated
986. By Antti Kaijanmäki

review adjustments

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> and more!

All fixed.

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> and more!

All fixed.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:986
http://jenkins.qa.ubuntu.com/job/unity8-ci/3705/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2829
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/707
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/799
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/799
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/799/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/799
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2910
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4072
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4072/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10777

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3705/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

I tested the packages in silo 001 wrt the simcard & icon changes in indicator-network. I haven't reviewed this diff at the code level, but can confirm the end features in i-network LGTM on an N4.

> * Did you perform an exploratory manual test run of the code change and any related functionality?

Yes, via indicator-network

> * Did CI run pass? If not, please explain why.

The individual ci systems succeeded, but https://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/707/console was marked as UNSTABLE.

Revision history for this message
Albert Astals Cid (aacid) wrote :

So what's missing for a top-approval?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:986
http://jenkins.qa.ubuntu.com/job/unity8-ci/3778/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3073
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/780
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/872
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/872
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/872/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/872
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3113
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4316
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4316/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11047

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3778/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

https://www.dropbox.com/home/Public/locked_sim

locked_sim1.png) this is how it comes up on boot. looks ok
locked_sim2.png) after unlocking phone. Note it still says "SIM Locked"
locked_sim3.png) restart indicator-network
locked_sim4.png) called the phone when it was in state 3. rejected
locked_sim5.png) called phone again. Can see the notification for accepting call, but see is still "Offline" & no strength icon
locked_sim6.png) restarted phone. came up without a locked sim icon in panel as in 1
locked_sim7.png) resarted again and unlocked. finally went into searching, then picked up a network. But it doesn't match with the designs you provided where there is the phone number above the carrier. There is no way to tell the sims apart if connected on same network.

review: Needs Fixing
Revision history for this message
Pete Woods (pete-woods) wrote :

Looks like you've posted an 'internal' dropbox link there. You need to get the sharing URL.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Sorry, stupid folders...

https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim1.png
https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim2.png
https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim3.png
https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim4.png
https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim5.png
https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim6.png
https://dl.dropboxusercontent.com/u/85539674/locked_sim/locked_sim7.png

5 was after a indicator-network restart

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

/usr/share/ofono/scripts/list-modems output for each state.

1) http://paste.ubuntu.com/7969273/
2) http://paste.ubuntu.com/7969275/
3) http://paste.ubuntu.com/7969276/
4) http://paste.ubuntu.com/7969279/
5) http://paste.ubuntu.com/7969280/
6) can't reproduce
7) http://paste.ubuntu.com/7969282/

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

committed a small fix to indicator-network branch. rebuild is on it's way.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

OK, the UI elements look ok, but the backend is not working.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Panel/Indicators/MenuItemFactory.qml'
2--- qml/Panel/Indicators/MenuItemFactory.qml 2014-07-10 13:36:41 +0000
3+++ qml/Panel/Indicators/MenuItemFactory.qml 2014-07-31 13:32:19 +0000
4@@ -57,6 +57,7 @@
5
6 "unity.widgets.systemsettings.tablet.wifisection" : wifiSection,
7 "unity.widgets.systemsettings.tablet.accesspoint" : accessPoint,
8+ "com.canonical.indicator.network.modeminfoitem" : modeminfoitem,
9 }
10
11 function getExtendedProperty(object, propertyName, defaultValue) {
12@@ -375,6 +376,79 @@
13 }
14
15 Component {
16+ id: modeminfoitem;
17+ ModemInfoItem {
18+ objectName: "modemInfoItem"
19+ property QtObject menuData: null
20+ property var menuModel: menuFactory.menuModel
21+ property int menuIndex: -1
22+ property var extendedData: menuData && menuData.ext || undefined
23+
24+ property var statusLabelAction: UnityMenuAction {
25+ model: menuModel
26+ index: menuIndex
27+ name: getExtendedProperty(extendedData, "xCanonicalModemStatusLabelAction", "")
28+ }
29+ statusText: statusLabelAction.valid ? statusLabelAction.state : ""
30+
31+ property var statusIconAction: UnityMenuAction {
32+ model: menuModel
33+ index: menuIndex
34+ name: getExtendedProperty(extendedData, "xCanonicalModemStatusIconAction", "")
35+ }
36+ statusIcon: statusIconAction.valid ? statusIconAction.state : ""
37+
38+ property var connectivityIconAction: UnityMenuAction {
39+ model: menuModel
40+ index: menuIndex
41+ name: getExtendedProperty(extendedData, "xCanonicalModemConnectivityIconAction", "")
42+ }
43+ connectivityIcon: connectivityIconAction.valid ? connectivityIconAction.state : ""
44+
45+ property var simIdentifierLabelAction: UnityMenuAction {
46+ model: menuModel
47+ index: menuIndex
48+ name: getExtendedProperty(extendedData, "xCanonicalModemSimIdentifierLabelAction", "")
49+ }
50+ simIdentifierText: simIdentifierLabelAction.valid ? simIdentifierLabelAction.state : ""
51+
52+ property var roamingAction: UnityMenuAction {
53+ model: menuModel
54+ index: menuIndex
55+ name: getExtendedProperty(extendedData, "xCanonicalModemRoamingAction", "")
56+ }
57+ roaming: roamingAction.valid ? roamingAction.state : false
58+
59+ property var unlockAction: UnityMenuAction {
60+ model: menuModel
61+ index: menuIndex
62+ name: getExtendedProperty(extendedData, "xCanonicalModemLockedAction", "")
63+ }
64+ onUnlock: {
65+ unlockAction.activate();
66+ }
67+ locked: unlockAction.valid ? unlockAction.state : false
68+
69+ onMenuModelChanged: {
70+ loadAttributes();
71+ }
72+ onMenuIndexChanged: {
73+ loadAttributes();
74+ }
75+
76+ function loadAttributes() {
77+ if (!menuModel || menuIndex == -1) return;
78+ menuModel.loadExtendedAttributes(menuIndex, {'x-canonical-modem-status-label-action': 'string',
79+ 'x-canonical-modem-status-icon-action': 'string',
80+ 'x-canonical-modem-connectivity-icon-action': 'string',
81+ 'x-canonical-modem-sim-identifier-label-action': 'string',
82+ 'x-canonical-modem-roaming-action': 'string',
83+ 'x-canonical-modem-locked-action': 'string'});
84+ }
85+ }
86+ }
87+
88+ Component {
89 id: messageItem
90
91 MessageMenuItemFactory {
92
93=== added file 'qml/Panel/Indicators/ModemInfoItem.qml'
94--- qml/Panel/Indicators/ModemInfoItem.qml 1970-01-01 00:00:00 +0000
95+++ qml/Panel/Indicators/ModemInfoItem.qml 2014-07-31 13:32:19 +0000
96@@ -0,0 +1,111 @@
97+/*
98+ * Copyright 2014 Canonical Ltd.
99+ *
100+ * This program is free software; you can redistribute it and/or modify
101+ * it under the terms of the GNU Lesser General Public License as published by
102+ * the Free Software Foundation; version 3.
103+ *
104+ * This program is distributed in the hope that it will be useful,
105+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
106+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
107+ * GNU Lesser General Public License for more details.
108+ *
109+ * You should have received a copy of the GNU Lesser General Public License
110+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
111+ */
112+
113+import QtQuick 2.0
114+import QtQuick.Layouts 1.1
115+import Ubuntu.Components 0.1
116+import Ubuntu.Components.ListItems 0.1 as ListItem
117+
118+ListItem.Empty {
119+ id: menu
120+ implicitHeight: mainColumn.implicitHeight
121+ + mainColumn.anchors.topMargin
122+ + mainColumn.anchors.bottomMargin
123+
124+ property alias statusIcon: statusIcon.name
125+ property alias statusText: labelStatus.text
126+ property alias connectivityIcon: iconConnectivity.name
127+ property alias simIdentifierText: labelSimIdentifier.text
128+ property bool locked : false
129+ property bool roaming: false
130+ signal unlock
131+
132+ ColumnLayout {
133+ id: mainColumn
134+ anchors.fill: parent
135+ anchors.margins: menu.__contentsMargins
136+ spacing: units.gu(0.5)
137+
138+ Label {
139+ id: labelSimIdentifier
140+ elide: Text.ElideRight
141+ visible: text !== ""
142+ font.bold: true
143+ opacity: menu.locked ? 0.6 : 1.0
144+ }
145+
146+ RowLayout {
147+ id: statusRow
148+ spacing: units.gu(1)
149+
150+ height: labelStatus.height
151+ width: parent.width
152+
153+ Label {
154+ id: labelStatus
155+ elide: Text.ElideRight
156+ opacity: 0.6
157+ }
158+
159+ RowLayout {
160+ spacing: 0.5
161+ height: parent.height
162+ Icon {
163+ id: statusIcon
164+ color: Theme.palette.selected.backgroundText
165+
166+ Layout.preferredHeight: parent.height
167+ Layout.preferredWidth: Layout.preferredHeight
168+
169+ visible: name !== ""
170+ }
171+
172+ Icon {
173+ id: iconConnectivity
174+ color: Theme.palette.selected.backgroundText
175+
176+ Layout.preferredHeight: parent.height
177+ Layout.preferredWidth: Layout.preferredHeight
178+
179+ visible: name !== ""
180+ }
181+ }
182+
183+ Item {
184+ // eat up all the excess space so that RoamingIndication
185+ // ends up to the far right
186+ Layout.fillWidth: true
187+ }
188+
189+ RoamingIndication {
190+ id: roamingIndication
191+ visible: menu.roaming
192+ Layout.fillHeight: true
193+ }
194+ }
195+
196+ Button {
197+ id: buttonUnlock
198+ objectName: "buttonUnlockSim"
199+ visible: menu.locked
200+
201+ text: i18n.tr("Unlock SIM")
202+ Layout.preferredWidth: implicitWidth + units.gu(5)
203+
204+ onTriggered: menu.unlock()
205+ }
206+ }
207+}
208
209=== added file 'qml/Panel/Indicators/RoamingIndication.qml'
210--- qml/Panel/Indicators/RoamingIndication.qml 1970-01-01 00:00:00 +0000
211+++ qml/Panel/Indicators/RoamingIndication.qml 2014-07-31 13:32:19 +0000
212@@ -0,0 +1,40 @@
213+/*
214+ * Copyright 2014 Canonical Ltd.
215+ *
216+ * This program is free software; you can redistribute it and/or modify
217+ * it under the terms of the GNU Lesser General Public License as published by
218+ * the Free Software Foundation; version 3.
219+ *
220+ * This program is distributed in the hope that it will be useful,
221+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
222+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
223+ * GNU Lesser General Public License for more details.
224+ *
225+ * You should have received a copy of the GNU Lesser General Public License
226+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
227+ */
228+
229+import QtQuick 2.0
230+import QtQuick.Layouts 1.1
231+import Ubuntu.Components 0.1
232+
233+RowLayout {
234+ spacing: units.gu(0.5)
235+ Label {
236+ id: labelRoaming
237+ elide: Text.ElideRight
238+ fontSize: "x-small"
239+ text: i18n.tr("Roaming")
240+ opacity: 0.6
241+ }
242+
243+ Icon {
244+ id: iconRoaming
245+ color: Theme.palette.selected.backgroundText
246+
247+ Layout.preferredHeight: parent.height
248+ Layout.preferredWidth: Layout.preferredHeight
249+
250+ name: "network-cellular-roaming"
251+ }
252+}
253
254=== modified file 'tests/qmltests/Panel/Indicators/tst_MenuItemFactory.qml'
255--- tests/qmltests/Panel/Indicators/tst_MenuItemFactory.qml 2014-07-07 11:07:55 +0000
256+++ tests/qmltests/Panel/Indicators/tst_MenuItemFactory.qml 2014-07-31 13:32:19 +0000
257@@ -112,6 +112,7 @@
258
259 { tag: 'wifisection', type: "unity.widgets.systemsettings.tablet.wifisection", objectName: "wifiSection" },
260 { tag: 'accesspoint', type: "unity.widgets.systemsettings.tablet.accesspoint", objectName: "accessPoint" },
261+ { tag: 'modeminfoitem', type: "com.canonical.indicator.network.modeminfoitem", objectName: "modemInfoItem" },
262
263 { tag: 'unknown', type: "", objectName: "standardMenu"}
264 ];
265@@ -542,6 +543,52 @@
266 compare(loader.item.enabled, data.enabled, "Enabled does not match data");
267 }
268
269+ function test_create_modemInfoItem_data() {
270+ // ModemInfoItem gets all it's data through the actions.
271+ return [
272+ {
273+ label: "",
274+ enabled: true,
275+ checked: false,
276+ statusLabelAction: "action::statusLabel",
277+ statusIconAction: "action::statusIcon",
278+ connectivityIconAction: "action::connectivityIcon",
279+ simIdentifierLabelAction: "action::simIdentifierLabel",
280+ roamingAction: "action::roaming",
281+ unlockAction: "action::unlock"
282+ }
283+ ];
284+ }
285+
286+ function test_create_modemInfoItem(data) {
287+ menuData.type = "com.canonical.indicator.network.modeminfoitem";
288+ menuData.label = data.label;
289+ menuData.sensitive = data.enabled;
290+ menuData.isToggled = data.checked;
291+ menuData.ext = {
292+ 'xCanonicalModemStatusLabelAction': data.statusLabelAction,
293+ 'xCanonicalModemStatusIconAction': data.statusIconAction,
294+ 'xCanonicalModemConnectivityIconAction': data.connectivityIconAction,
295+ 'xCanonicalModemSimIdentifierLabelAction': data.simIdentifierLabelAction,
296+ 'xCanonicalModemRoamingAction': data.roamingAction,
297+ 'xCanonicalModemLockedAction': data.unlockAction,
298+ };
299+
300+ loader.data = menuData;
301+ loader.sourceComponent = factory.load(menuData);
302+ tryCompareFunction(function() { return loader.item != undefined; }, true);
303+ compare(loader.item.objectName, "modemInfoItem", "Should have created a modem info item.");
304+ compare(loader.item.text, data.label, "Label does not match data");
305+
306+ compare(loader.item.statusLabelAction.name, data.statusLabelAction, "StatusLabel action incorrect");
307+ compare(loader.item.statusIconAction.name, data.statusIconAction, "StatusIcon action incorrect");
308+ compare(loader.item.connectivityIconAction.name, data.connectivityIconAction, "ConnectivityIcon action incorrect");
309+ compare(loader.item.simIdentifierLabelAction.name, data.simIdentifierLabelAction, " action incorrect");
310+ compare(loader.item.roamingAction.name, data.roamingAction, "Roaming action incorrect");
311+ compare(loader.item.unlockAction.name, data.unlockAction, "Unlock action incorrect");
312+ compare(loader.item.enabled, data.enabled, "Enabled does not match data");
313+ }
314+
315 function test_create_messageItem() {
316 menuData.type = "com.canonical.indicator.messages.messageitem";
317

Subscribers

People subscribed via source and target branches