Fix: Hide notebook(if present) on fullscreen

sugarlabs / sugar-toolkit-gtk3

Sugar Learning Environment, Activity Toolkit, GTK 3.

GNU Lesser General Public License v2.1

21 stars 80 forks source link

Fix: Hide notebook(if present) on fullscreen #381

Closed rhl-bthr closed 6 years ago

rhl-bthr commented 6 years ago

Fixed in this PR: Terminal Activity #25

Effect of this PR:

fullscreen will hide notebook tabs(if present)
unfullscreen will show notebook tabs

@quozl, I am in doubt whether this change should have been implemented in the toolkit, or local to the terminal activity. My opinion: Fullscreen implies the current working space should be the only thing present on the entire screen. Hence, this change should be there for all activities, in the toolkit itself

Tested on: terminal activity, Sugar 0.112 (rdesktop, Ubuntu 16.04)

rhl-bthr commented 6 years ago

If these changes are approved, kindly let me know before merging, as the following changes also need to be done.

[ ] checking all activities to see if this change can affect their intended implementation
[ ] If found, make changes in those activities, or in the implementation here.
[ ] activities using Notebook at relevant places should have instance name as _notebook
[ ] making necessary changes in the developer docs
[ ] Adding notebook in this list. (Though, I am unsure of the order)

quozl commented 6 years ago

@Pro-Panda, thanks for the research. It does begin to look complicated. I've solved it in Terminal by adding methods to the activity class which use the superior methods. https://github.com/sugarlabs/terminal-activity/commit/b37d83ff2f16717cb3dad08590f3c3c56f774232

rhl-bthr commented 6 years ago

@quozl, Thank you 😄 But shouldn't this be a global feature, pertaining to all activities ?

Eg: pydebug also has a fullscreen option, which doesn't hide the notebook (although tabs are at the bottom, and don't overlap with the unfullscreen button, but tabs should be hidden in fullscreen)

quozl commented 6 years ago

Thanks. I've not used or shipped pydebug.

I'm not sure if it is justified to make it global; there are so few activities that embed a Gtk.Notebook as activity canvas.

https://github.com/search?utf8=%E2%9C%93&q=org%3Asugarlabs+set_canvas%28self._notebook%29&type=Code is a useful search.

Activities that I'm caring for that have a Gtk.Notebook;

Browse; yet to be fixed, https://github.com/sugarlabs/browse-activity/issues/69
Speak; does not use tabs,
Terminal; now fixed,
Pippy; does not have a fullscreen button.

rhl-bthr commented 6 years ago

Thank you for the explanation. I too agree now, that making it global is not justified

My mistake was looking just for instances of Notebook (here) before opening the PR.