Add History to Tk Viewer

[jbdyn]

As the title says.

You can now go backward and forward in time.
Also, the button styles were changed.

Fixes: #82

[Debilski]

Ahem, don’t get too excited with the unicode characters …

(And please use the BUTTON_STYLE dict for styling, see #882.)

Otherwise looks like a cool thing (I don’t have the time right now to look at it in more detail), thanks.

[otizonaizit]

Oh, on which system do you see the font problems?

Regarding the styling dictionary: you can't style the font of the labels there, because at definition time there's still no Tk instance running, so you can't access the Tk default font handle. Definitely a ugly thing, but I don't see how to do it differently.

This is in general a huge improvement to the GUI and one of the most requested still missing features. I'm super excited to see this going in before the next ASPP.

@Debilski : do you think we can merge as-is? If not, what do you think is missing?

[otizonaizit]

@jbdyn : maybe instead of unicode characters for the navigation controls we could use images.

As the story about using custom fonts in Tk is very, but very very very sad, I think this is the only reasonable option left?

Looking at this and at this it seems it should be straightforward to load an image with pillow and stick it into the button instead of the text label. What do you think?

[jbdyn]

As the story about using custom fonts in Tk is very, but very very very sad, I think this is the only reasonable option left?

After some research, I can confirm. I had no idea ... 👀

[...] it should be straightforward to load an image with pillow and stick it into the button instead of the text label [...]

For now, this also works without using pillow (had issues with module-not-found errors, btw).

With 5b087a9 it looks like this:

BUT: These images have to be rasterized files, so fixed in size (now PNG, 32x32 px) 🥶 . This could be a problem on HiDPI displays; have not tested it.
We might need to open the images in a PIL.Image object, then resize it according to the dpi of the screen and then pass that to the tkinter.PhotoImage class. Or use PIL.Imagetk.PhotoImage (or similar) 🤷

And I labelled the quit button with QUIT again as I could not figure out a good icon for it. The power symbol we had before is like an "on / off", which is not really on point as it just turns "off", and an "X" just looked odd down there. 😅

[otizonaizit]

Great, this solves the problem then!

BUT: These images have to be rasterized files, so fixed in size (now PNG, 32x32 px) 🥶 . This could be a problem on HiDPI displays; have not tested it. We might need to open the images in a PIL.Image object, then resize it according to the dpi of the screen and then pass that to the tkinter.PhotoImage class. Or use PIL.Imagetk.PhotoImage (or similar) 🤷

@Debilski was experimenting with scalable fonts+canvas in #910, so maybe we can revisit the icon scaling once those experiments show the way to go? But this is not a blocker for this PR in my opinion.

And I labelled the quit button with QUIT again as I could not figure out a good icon for it. The power symbol we had before is like an "on / off", which is not really on point as it just turns "off", and an "X" just looked odd down there. 😅

Ah, OK. If I remember correctly @Debilski had the same comment some time ago on the use of that symbol to mean "switch off"?

I personally don't think that "on/off" is ambiguous: it is immediately clear to me what the button is supposed to do, and in the worst case you understand and memorize the first time you hit it by thinking that it will start a new game? :-))

But I am not attached to having an icon for it, we can leave it with "QUIT". In my comment #882 (comment) on #882 I was even arguing to remove the button completely (and just a couple of months later I was arguing to keep it at any cost while talking with you on the train...). By the way, the idea of using the symbols for navigation was discussed already in #882. I am happy to see it realized now :-)

[Debilski]

Should we maybe rebase the styling things into a new PR? Accepting the history viewer seems pretty uncontroversial and can be done already. (I am away for the week and cannot try out a lot.)

My problems with the icons are that it may make the buttons too small (not sure if a real issue but students are using this on a laptop that they typically don’t know well). The other is that the UI element that is a (action) button is not used for icons, no? Sole icons can go to a title bar/menu area but usually not anywhere else. (I am open to learn otherwise from examples.)

[jbdyn]

Should we maybe rebase the styling things into a new PR?

That is a good idea, as icons seem to come with new problems (so unfortunate ...).

I would revert to simple labels previous, play/pause, next, slower and faster for now and the history functionality can be merged.

In a dedicated PR about the button looks, we can do

• icons,
• icon sizes,
• button styles,
• and probably also button arrangement etc.

without the need to hurry.

Sounds good?

[otizonaizit]

Should we maybe rebase the styling things into a new PR?

That is a good idea, as icons seem to come with new problems (so unfortunate ...).

I would revert to simple labels previous, play/pause, next, slower and faster for now and the history functionality can be merged.

In a dedicated PR about the button looks, we can do

* icons,

* icon sizes,

* button styles,

* and probably also button arrangement etc.

without the need to hurry.

Sounds good?

Yes, so once you are done with the rebase I can merge this!

[jbdyn]

Okay, I rewrote history a bit and the UI looks like this now:

Functionally, nothing changed, of course. Should be ready to merge now.