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 686938 - Exit fullscreen button uses the wrong icon
Exit fullscreen button uses the wrong icon
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
3.6.x
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-10-26 11:55 UTC by Allan Day
Modified: 2016-03-31 14:01 UTC
See Also:
GNOME target: ---
GNOME version: 3.5/3.6

Attachments
Emit proper notifications for App.fullscreen (1.65 KB, patch)
2012-10-26 14:01 UTC, Alexander Larsson 		committed Details | Review
Use the right icon for the fullscreen button (1.34 KB, patch)
2012-10-26 14:01 UTC, Alexander Larsson 		committed Details | Review

Description Allan Day 2012-10-26 11:55:49 UTC 
The exit fullscreen button - which is in the fullscreen toolbar - uses the view-fullscreen-symbolic icon. This is wrong - the button does not enter fullscreen, it exits it. Something like view-restore-symbolic would be a better choice.
Comment 1 Alexander Larsson 2012-10-26 14:01:12 UTC 
Created attachment 227363 [details] [review]
Emit proper notifications for App.fullscreen

We don't want to send notify emissions when we set the fullscreen
property as that only requests an async change (reading the property
will not have changed value yet). So, we disable automatic notifcation
for the property and notify manually when the real state changes.
Comment 2 Alexander Larsson 2012-10-26 14:01:15 UTC 
Created attachment 227364 [details] [review]
Use the right icon for the fullscreen button

We were always using the go-to-fullscreen icon, but
we should use the restore-from-fullscreen icon when in fullscreen
mode already.
Comment 3 Zeeshan Ali 2012-10-26 14:42:05 UTC 
Review of attachment 227364 [details] [review]:

ACK
Comment 4 Zeeshan Ali 2012-10-26 14:45:47 UTC 
Review of attachment 227363 [details] [review]:

otherwise good

::: src/app.vala
@@ +407,3 @@
             return false;
         });
+        window.window_state_event.connect ((ev) => {

nitpick: ev -> event

@@ +408,3 @@
         });
+        window.window_state_event.connect ((ev) => {
+            if ((ev.changed_mask & WindowState.FULLSCREEN) != 0)

this could be written as: if (WindowState.FULLSCREEN in ev.changed_mask)
Comment 5 Alexander Larsson 2012-10-26 15:01:50 UTC 
Attachment 227363 [details] pushed as cc64e33 - Emit proper notifications for App.fullscreen
Attachment 227364 [details] pushed as 9b07acb - Use the right icon for the fullscreen button