After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 763259 - Location access: various UI improvements
Location access: various UI improvements
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Privacy
3.19.x
Other Linux
: Normal normal
: ---
Assigned To: Rui Matos
Control-Center Maintainers
: 763107 763261 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-03-07 18:51 UTC by Allan Day
Modified: 2016-03-16 12:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
privacy: Fix margins around location dialog widgets (2.50 KB, patch)
2016-03-09 20:08 UTC, Zeeshan Ali
committed Details | Review
privacy: Better description for 'Location Services' (1.35 KB, patch)
2016-03-09 20:08 UTC, Zeeshan Ali
accepted-commit_after_freeze Details | Review
close up screenshot (13.46 KB, image/png)
2016-03-10 09:24 UTC, Allan Day
  Details
privacy: Better description for 'Location Services' (1.33 KB, patch)
2016-03-10 14:24 UTC, Zeeshan Ali
committed Details | Review
privacy: Vertically center "Location Services" label (1.09 KB, patch)
2016-03-10 14:24 UTC, Zeeshan Ali
committed Details | Review
privacy: Only disable apps switches (3.48 KB, patch)
2016-03-10 14:24 UTC, Zeeshan Ali
committed Details | Review

Description Allan Day 2016-03-07 18:51:50 UTC
This is a collection of UI issues with the new location access dialog in the privacy panel.

 1. Spacing is off inside the dialog. I'd suggest adding 12px margin-bottom to location-vbox and 12px margin to location_dialog

 2. "Location Services" label isn't vertically centered inside the list row.

 3. The idea in the mockups was to have the application switches lose their colour when location services is off, rather than making the insensitive. This would allow a user to change individual application settings while location services are off globally.

 4. When the location switch is off, there's no need to dim the entire applications section. It would be better just to change the colour of the switches, as described above.

 5. The long description string could be improved. Issues with it:
  * "WiFi" should be "Wi-Fi"
  * We generally say "mobile broadband" rather than "GPS" in settings.
  * Could be confusing if the device doesn't have Wi-Fi or GPS.
  * "enabling" is a sucky word. The second sentence also implies that they are turned off.
  * "determine" isn't everyday language.

A better label might be something like: "Location services allow applications to know your location. Having Wi-Fi and mobile broadband switched on increases accuracy." This assumes that the device has Wi-Fi and mobile broadband with GPS - can we detect that and change the string accordingly?
Comment 1 Zeeshan Ali 2016-03-08 11:04:11 UTC
Well I already filed bug#763107 for this but since you provided more details here, I'll mark that as dup.
Comment 2 Zeeshan Ali 2016-03-08 11:06:55 UTC
*** Bug 763107 has been marked as a duplicate of this bug. ***
Comment 3 Zeeshan Ali 2016-03-08 11:10:49 UTC
(In reply to Allan Day from comment #0)
>
> A better label might be something like: "Location services allow
> applications to know your location. Having Wi-Fi and mobile broadband
> switched on increases accuracy." This assumes that the device has Wi-Fi and
> mobile broadband with GPS - can we detect that and change the string
> accordingly?

Yes but unless some code in control-centre is already doing that, it won't be a simple fix and I won't be able to fix that in 3.20.
Comment 4 Zeeshan Ali 2016-03-08 11:13:01 UTC
*** Bug 763261 has been marked as a duplicate of this bug. ***
Comment 5 Zeeshan Ali 2016-03-08 11:14:39 UTC
Also there is redundant padding at the right side of application access switches as reported by Cosimo in bug#763261.
Comment 6 Zeeshan Ali 2016-03-09 19:37:27 UTC
(In reply to Allan Day from comment #0)
>
>  2. "Location Services" label isn't vertically centered inside the list row.

Are you sure about this? I see it completely centered (vertically) here and also i don't see any reason why it won't be (looking at the .ui definition).
Comment 7 Zeeshan Ali 2016-03-09 20:08:06 UTC
Created attachment 323547 [details] [review]
privacy: Fix margins around location dialog widgets
Comment 8 Zeeshan Ali 2016-03-09 20:08:12 UTC
Created attachment 323548 [details] [review]
privacy: Better description for 'Location Services'

New string from Allan Day.
Comment 9 Zeeshan Ali 2016-03-09 20:11:39 UTC
(In reply to Allan Day from comment #0)
> 
>  3. The idea in the mockups was to have the application switches lose their
> colour when location services is off, rather than making the insensitive.
> This would allow a user to change individual application settings while
> location services are off globally.
> 
>  4. When the location switch is off, there's no need to dim the entire
> applications section. It would be better just to change the colour of the
> switches, as described above.

I'm not so sure it makes sense to let user chance some settings when they have no effect. Still if you think it's a good idea, I'd go with it but until it's actually possible to change the colors (bug#725648 ?), I'd want to keep the current behaviour.
Comment 10 Allan Day 2016-03-10 09:24:18 UTC
Created attachment 323595 [details]
close up screenshot

(In reply to Zeeshan Ali (Khattak) from comment #6)
> (In reply to Allan Day from comment #0)
> >
> >  2. "Location Services" label isn't vertically centered inside the list row.
> 
> Are you sure about this? ...

Yes. It is clearly not centered - see the screenshot.
Comment 11 Allan Day 2016-03-10 09:26:25 UTC
(In reply to Zeeshan Ali (Khattak) from comment #3)
> (In reply to Allan Day from comment #0)
> >
> > A better label might be something like: "Location services allow
> > applications to know your location. Having Wi-Fi and mobile broadband
> > switched on increases accuracy." This assumes that the device has Wi-Fi and
> > mobile broadband with GPS - can we detect that and change the string
> > accordingly?
> 
> Yes but unless some code in control-centre is already doing that, it won't
> be a simple fix and I won't be able to fix that in 3.20.

In that case, it might have to be something like this:

"Location services allow applications to know your location. Using Wi-Fi and mobile broadband increases accuracy."

It would be better not to mention Wi-Fi and mobile broadband if it isn't available, of course.
Comment 12 Bastien Nocera 2016-03-10 09:35:48 UTC
Review of attachment 323547 [details] [review]:

Sure.
Comment 13 Bastien Nocera 2016-03-10 09:37:06 UTC
Review of attachment 323548 [details] [review]:

Fine, will need to ask for freeze break though.
Comment 14 Allan Day 2016-03-10 10:08:09 UTC
(In reply to Zeeshan Ali (Khattak) from comment #9)
> (In reply to Allan Day from comment #0)
> > 
> >  3. The idea in the mockups was to have the application switches lose their
> > colour when location services is off, rather than making the insensitive.
> > This would allow a user to change individual application settings while
> > location services are off globally.
> > 
> >  4. When the location switch is off, there's no need to dim the entire
> > applications section. It would be better just to change the colour of the
> > switches, as described above.
> 
> I'm not so sure it makes sense to let user chance some settings when they
> have no effect. 

It's true that the practical benefit is somewhat marginal. There is a logical argument for the change though: I've filed bug 763443 for this.

> Still if you think it's a good idea, I'd go with it but
> until it's actually possible to change the colors (bug#725648 ?), I'd want
> to keep the current behaviour.

There's still no need to dim the entire applications section - it doesn't communicate anything new, and makes the UI harder to read. Just make the switches insensitive when location services are off.
Comment 15 Zeeshan Ali 2016-03-10 14:24:26 UTC
Created attachment 323635 [details] [review]
privacy: Better description for 'Location Services'

New string from Allan Day.
Comment 16 Zeeshan Ali 2016-03-10 14:24:43 UTC
Created attachment 323636 [details] [review]
privacy: Vertically center "Location Services" label
Comment 17 Zeeshan Ali 2016-03-10 14:24:51 UTC
Created attachment 323637 [details] [review]
privacy: Only disable apps switches

Let's not disable the whole Applications section when "Location Services"
are disabled but rather only the individual app permission switches.
Comment 18 Bastien Nocera 2016-03-10 14:29:57 UTC
Review of attachment 323548 [details] [review]:

Adding patch status
Comment 19 Bastien Nocera 2016-03-10 14:30:13 UTC
Review of attachment 323636 [details] [review]:

Yep
Comment 20 Bastien Nocera 2016-03-10 14:31:07 UTC
Review of attachment 323637 [details] [review]:

Looks good.
Comment 21 Bastien Nocera 2016-03-10 14:31:41 UTC
Review of attachment 323635 [details] [review]:

Patch status again.
Comment 22 Zeeshan Ali 2016-03-11 13:44:52 UTC
Attachment 323547 [details] pushed as 76e3a53 - privacy: Fix margins around location dialog widgets
Attachment 323635 [details] pushed as a2ab6d4 - privacy: Better description for 'Location Services'
Attachment 323636 [details] pushed as d0c1ed6 - privacy: Vertically center "Location Services" label
Attachment 323637 [details] pushed as 48c66a5 - privacy: Only disable apps switches
Comment 23 Allan Day 2016-03-16 10:20:52 UTC
(In reply to Zeeshan Ali (Khattak) from comment #16)
> Created attachment 323636 [details] [review] [review]
> privacy: Vertically center "Location Services" label

The label isn't centered in the screenshot that's in the release notes. Are you sure this issue got fixed?
Comment 24 Zeeshan Ali 2016-03-16 12:44:54 UTC
(In reply to Allan Day from comment #23)
> (In reply to Zeeshan Ali (Khattak) from comment #16)
> > Created attachment 323636 [details] [review] [review] [review]
> > privacy: Vertically center "Location Services" label
> 
> The label isn't centered in the screenshot that's in the release notes. Are
> you sure this issue got fixed?

Not sure where release notes screenshots come from and if it's up2date but I showed the UI before and after the change to Richard Hughes and he verified that it did it's job. TBH, I can't tell on my own. :(