Opened 12 years ago

Closed 10 years ago

Last modified 8 years ago

#347 closed defect (fixed)

Pippy does not function properly at non-XO screen resolutions

Reported by: lfaraone Owned by: cjb
Priority: High Milestone:
Component: Pippy Version: Unspecified
Severity: Major Keywords:
Cc: sascha_silbe, jasonhoekstra, brian, eben, garycmartin, joleary, jpichon, m_anish Distribution/OS: Ubuntu
Bug Status: Resolved

Description

Forwarded from https://bugs.edge.launchpad.net/ubuntu/+source/sugar-pippy-activity/+bug/307179

"screen too small on left hand side... text for 'Examples' cuts off, making them unreadable."

Please also create a separate component for Pippy

Attachments (3)

pippy_res_347.patch (2.4 KB) - added by joleary 11 years ago.
button size and panes for different resolutions
pippy_res_347_v2.patch (4.0 KB) - added by joleary 11 years ago.
patch using style.GRID_CELL_SIZE
table-text (28.3 KB) - added by m_anish 10 years ago.
test file, ignore!

Download all attachments as: .zip

Change History (29)

comment:1 Changed 12 years ago by lfaraone

  • Bug Status changed from Unconfimed to New
  • Component changed from sugar to Pippy
  • Distribution/OS changed from Unspecified to Ubuntu
  • Owner changed from marcopg to lfaraone

comment:2 Changed 12 years ago by lfaraone

  • Owner changed from lfaraone to cjb
  • Status changed from new to assigned

comment:3 Changed 12 years ago by sascha_silbe

  • Cc sascha_silbe added

comment:4 Changed 12 years ago by jasonhoekstra

  • Cc jasonhoekstra added

comment:5 Changed 12 years ago by brian

  • Cc brian added

What's the status of this? Is this fixed with v33?

http://activities.sugarlabs.org/en-US/sugar/downloads/latest/4041

comment:6 Changed 12 years ago by erikos

Pippy still has a lot of XO screen resolution dependent code. For example the 'Bounce' example. I am sure you will find others when you grab for "1200". This can be made screen size dependent gtk.gdk.screen_width(). As well the Run, Stop, Clear buttons are quite big on a bigger screen.

comment:7 Changed 11 years ago by tomeu

  • Milestone changed from Unspecified by Release Team to 0.86
  • Priority changed from Unspecified by Maintainer to High
  • Severity changed from Unspecified to Major

If you cannot make the UI layout really scalable, consider "hardcoding" the sizes based on screen cells: http://wiki.sugarlabs.org/go/Human_Interface_Guidelines/The_Sugar_Interface/Layout_Guidelines

comment:8 follow-up: Changed 11 years ago by joleary

I have a patch for this that sets the buttons at a fixed width and uses a vpane so the size of the top and bottom areas can be resized. I have some questions though:

1) Is this acceptable, would a vpane suit the UI
2) Should the gripper for the vpane go above or below the buttons
3) What is a sane default pixel height for the buttons (is it specified in the UI guidelines)

comment:9 in reply to: ↑ 8 Changed 11 years ago by tomeu

  • Cc eben garycmartin added

Replying to joleary:

I have a patch for this that sets the buttons at a fixed width and uses a vpane so the size of the top and bottom areas can be resized. I have some questions though:

1) Is this acceptable, would a vpane suit the UI

I personally like this option, adding some people to CC with more relevant opinions.

2) Should the gripper for the vpane go above or below the buttons

I think that the buttons should go in the top toolbar, like in the other activities.

3) What is a sane default pixel height for the buttons (is it specified in the UI guidelines)

If it goes in the toolbar, it will have the expected size (one grid cell).

As an aside, you can get the size in pixels of a cell with:

from sugar.graphics import style
...
style.GRID_CELL_SIZE

comment:10 follow-up: Changed 11 years ago by eben

Yes, a vpane definitely suits this UI. I also agree with Tomeu: The buttons should appear within the toolbar, so that the vpane cleanly splits the source and output sections.

It would be nice if these were just icon buttons. In fact, "run" and "stop" could just be a single button that changes state when toggled. The "play" button could also use the standard play icon, so it matches the appearance of the stop button being used. Maybe clear should remain a text button, though, so its action is made explicit (run and stop are more self-evident).

comment:11 in reply to: ↑ 10 Changed 11 years ago by garycmartin

Replying to eben:

Yes, a vpane definitely suits this UI. I also agree with Tomeu: The buttons should appear within the toolbar, so that the vpane cleanly splits the source and output sections.

It would be nice if these were just icon buttons. In fact, "run" and "stop" could just be a single button that changes state when toggled. The "play" button could also use the standard play icon, so it matches the appearance of the stop button being used.

+1, all sounds much better that what we have just now.

Maybe clear should remain a text button, though, so its action is made explicit (run and stop are more self-evident).

FWIW: I find both the "clean" button, and the examples tree-view very dangerous. Both of them act as an instant "wipe all your work" on a single click, and there is no warning alert, though undo keyboard shortcut does at least work after pressing the clean button. It means you need to frequently click "Keep", or copy your source to the clipboard buffer for safety. Perhaps a vpane could also be added to the examples tree-view so a user can at least fold it away to prevent an accidental click erasing their work (and make better use of non-XO screen resolutions)?

comment:12 Changed 11 years ago by walter

Or perhaps the current work could at least be pushed to the clipboard before being overwritten?

comment:13 follow-up: Changed 11 years ago by cjb

This bug is about screen resolution, not saving work.

That said, when you do click on an example in the tree-view, your work is never lost -- before we load the new example, we save the current source buffer as a new Journal entry. So, you can always find your old work there.

comment:14 follow-up: Changed 11 years ago by walter

CJB: Never knew that. Even better. I'll have to check my Journal to find all my code I accidentally lost last night :) Thanks for the heads up.

comment:15 in reply to: ↑ 13 Changed 11 years ago by garycmartin

Replying to cjb:

This bug is about screen resolution, not saving work.

Yep, hence my comment about having the tree-view also as an adjustable pane would allow a user to make better use of their available screen space :-)

That said, when you do click on an example in the tree-view, your work is never lost -- before we load the new example, we save the current source buffer as a new Journal entry. So, you can always find your old work there.

Wow now that is news to me. Have never seen this working myself, and it's not working for sure in Pippy-35 and Sugar 0.86. FWIW what is really needed is to have the standard Edit toolbar (copy/paste, undo/redo), these functions all work now but are not visually exposed so I doubt many kids know the key shortcuts, switching example could then just simply be included in the undo chain for folks to recover from (just like 'clear' does). Writing secret snapshots to the Journal seems rather unexpected and undiscoverable (I'd likely have thought it a bug if I'd ever seen it).

comment:16 in reply to: ↑ 14 Changed 11 years ago by garycmartin

Replying to walter:

CJB: Never knew that. Even better. I'll have to check my Journal to find all my code I accidentally lost last night :) Thanks for the heads up.

Sorry perhaps time for a new ticket, but FWIW I just tested pippy-35 in 0.86 and 0.84, and pippy-30 in 0.82, no extra Journal entries were created after writing some test code and then clicking on an example (or modifying one example and then clicking another one). Apologies to joleary and lfaraone for noising up their ticket.

comment:17 follow-up: Changed 11 years ago by joleary

Hi, sorry for the absence. So going on the feedback I need to change the path so it has a vpane and hpane to resize the contents properly. I should also move the buttons to the top toolbar and make the play/stop a toggle, and use standard icons for them.

That's no problem at all. I'd like it if someone would code review though, so who should I send the patch to before submitting?

comment:18 in reply to: ↑ 17 Changed 11 years ago by garycmartin

Replying to joleary:

Hi, sorry for the absence. So going on the feedback I need to change the path so it has a vpane and hpane to resize the contents properly. I should also move the buttons to the top toolbar and make the play/stop a toggle, and use standard icons for them.

Fab :-)

That's no problem at all. I'd like it if someone would code review though, so who should I send the patch to before submitting?

I think if you're after review, the best process is to attach your .patch file here to the ticket. Trac understands the format and shows it nicely formatted.

comment:19 Changed 11 years ago by joleary

  • Cc joleary added

Changed 11 years ago by joleary

button size and panes for different resolutions

comment:20 Changed 11 years ago by joleary

Just attached a patch for the squashed buttons and paned view. I'll do the menu buttons in another patch, would you like that attached here or should it put as a separate work item? I was planning something like below for a toolbox approach with the buttons:

1) remove the buttons from the current position
2) create a second toolbox containing the play / stop toggle button and clear button.
3) creating a small menu underneath to switch between activity view and pippy view (following the convention by other apps)

I hard-coded the button size for this patch, but in the toolbox patch I will keep them to grid cell size.

comment:21 Changed 11 years ago by tomeu

 	177	        gobutton.set_size_request(650, 45)

Using pixel sizes will break the layout in some screens. If we really need to specify fixed sizes, it's better to specify them in multiples of grid cells, see here for an explanation:

http://wiki.sugarlabs.org/go/Human_Interface_Guidelines/The_Sugar_Interface/Layout_Guidelines

So if we want a button to always be one cell wide and tall:

 	177	        gobutton.set_size_request(style.GRID_CELL_SIZE, style.GRID_CELL_SIZE)

Changed 11 years ago by joleary

patch using style.GRID_CELL_SIZE

comment:22 Changed 11 years ago by joleary

Just attached a new patch, it uses style.GRID_CELL_SIZE for the buttons. I made them all uniform in size, which leaves ugly spacing, but once they move to the toolbox it will be less of a problem. I also noticed that the main panels (editor, terminal and tree list) were assigned pixel sizes, so I changed them to GRID_CELL_SIZE to. Those widgets will resize anyway because of the containers they were in anyway, but I guess a move towards consistency wouldn't be to bad.

comment:23 Changed 11 years ago by jpichon

  • Cc jpichon added

Would anyone have objections if I push joleary's latest patch into the mainline? I realise the whole button matter is still unresolved for now, but the patch fixes the initial issue where Pippy is unusable on some configurations/resolutions. Tickets #937 and #1927 also seem to be reporting the same problem.

comment:24 Changed 11 years ago by m_anish

  • Cc m_anish added

I believe the screen resolution issues #1927 and examples not scaling to screen sizes olpc#9260 have been patched upstream. The existing issue that remains is the Share, Keep buttons fall off the main toolbar on <=0.84+xo1 olpc#9181. Its fix will hopefully resolve this bug.

comment:25 Changed 10 years ago by m_anish

  • Bug Status changed from New to Resolved
  • Resolution set to fixed
  • Status changed from assigned to closed

The latest pippy version (38) fixes this problem.

Changed 10 years ago by m_anish

test file, ignore!

comment:26 Changed 8 years ago by dnarvaez

  • Milestone 0.86 deleted

Milestone 0.86 deleted

Note: See TracTickets for help on using tickets.