Code review comment for lp:~unity-api-team/unity8/modeminfo

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.

« Back to merge proposal