Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to GTK4 #723

Closed
wants to merge 87 commits into from
Closed

Conversation

ranchester2
Copy link
Contributor

@ranchester2 ranchester2 commented Nov 5, 2021

Port GTG to GTK4 while keeping things the same. Highly recommended to not look at the entire diff but go through the commits individually.

Mentioning issue: #533

Build with liblarch from here: https://github.com/ranchester2/liblarch/tree/gtk4
Liblarch changes are very minimal, however drag and drop will need to be changed a bit when it starts working in GTK

Mostly works, existing issues:

  • Figure out how to recolor symbolic icons for dark mode in cell_renderer_tags NOTE: fix adds dependency on master sdk, which isn't available on Flathub, an alternative would be to bundle the latest GTK instead
  • cell_renderer_tags is slightly blurry this is also an issue in master, not a gtk4 issue
  • Harder to open right click menu for multi selection, you now have to shift+select and shift+right click instead of just right click
  • .menu-disclose
  • tags sidebar slightly tight spacing
  • plugin about dialog markup
  • Figure out treeview drag and drop (EDIT: Treeview DND is completely broken in GTK4, GTK4 bug prevents this).

I think this is quite ready currently, however there is the treeview dnd issue that can't really be fixed at the moment.
And this now uses the master SDK as GTK 4.5 is needed, this prevents releasing on Flathub with this until GNOME 42 is released (or bundle GTK)

Plugins:

  • Export and print
  • Send tasks via email
  • Untouched tasks
  • Urgency color
  • Hamster
  • Dev console
  • Gamify

Window positions aren't restored, GTK4 removed this feature as the api is now based on Wayland, not X11.

I added a "Today" button to the startdate calendar as you can no longer receive events about selection of the currently selected date in GtkCalendar (nor can you not have any date selected)

gtg-gtk4-screenshot

@Neui
Copy link
Contributor

Neui commented Nov 5, 2021

Nice! I looked only at some commits for now, but they seemed fine. The commit messages are the cherry on top.
I'd probably not merge this for the next release because this will take a while to review and test. (And this is still a draft PR for I just noticed)
Also, I wonder if some early stuff can be applied to the current GTK3 version now so some of the changes can already be used and smaller chunks are a bit easier to review. Like making the TagEditor an GtkDialog now (I don't remember why I didn't do that...)

@ranchester2
Copy link
Contributor Author

Nice! I looked only at some commits for now, but they seemed fine. The commit messages are the cherry on top. I'd probably not merge this for the next release because this will take a while to review and test. (And this is still a draft PR for I just noticed) Also, I wonder if some early stuff can be applied to the current GTK3 version now so some of the changes can already be used and smaller chunks are a bit easier to review. Like making the TagEditor an GtkDialog now (I don't remember why I didn't do that...)

All commits before the update imports commit were for "preparing", and are for GTK3. (However the gmenu commit didn't port the hamster plugin). However I missed quite a lot of stuff there, as PyGObject seems to be really bad at printing deprecation warnings, so I fixed those things later. Later invisible changes could be backported probably, however probably not as-is

@ranchester2
Copy link
Contributor Author

ranchester2 commented Nov 8, 2021

The main remaining issue with this is currently treeview drag and drop.

And that can't be really fixed as GTK broke it when inventing the new drag and drop api and it currently just doesn't work:

https://gitlab.gnome.org/GNOME/gtk/-/issues/3649
https://gitlab.gnome.org/GNOME/gtk/-/issues/4206
https://gitlab.gnome.org/GNOME/gtk/-/issues/3656
https://gitlab.gnome.org/GNOME/gtk/-/issues/3642

Maybe there should be an alternative way of adding parents for tags in the sidebar or reordering in the tree? However that will definitely be worse to use.

EDIT: the PyGObject override issue too, however that isn't really a problem and that override isn't more convenient in GTK4 anyway. That fix is basically done, just needs to be merged, the real issue is that in GTK itself it is broken.

@Neui
Copy link
Contributor

Neui commented Nov 8, 2021

On a related thing, it seems PyGObject also needs some fixes for gtk4 TreeView. Also I don't think you can "reoder" stuff anyway currently (minus parenting and un-parenting), but looking at your issue pages DnD looks like to be broken completely? We could hope that when we finally merge this PR that it'll be fixed by then...

Diegogangl started a branch https://github.com/getting-things-gnome/gtg/tree/ui_listboxes to migrate away from the TreeView widget, but doesn't seem to be complete however (and would need testing etc.)

(I am also wondering how keyboard-only users do parenting and unparenting?)

@ranchester2
Copy link
Contributor Author

A possible solution since there seem to be plans to drop treeview anyway, would maybe be to never actually fix treeview drag and drop, and instead not merge this until a port to the new list widgets is done? And then reimplement drag and drop with the non-treeview widgets that aren't broken. However that will definitely require very major changes.

@diegogangl
Copy link
Contributor

Hey @ranchester2, thanks for working on this! It really takes a load off our backs :)
Shame treeviews are being a problem again. Like @Neui said we don't support manual re-ordering, so DnD is only for parenting. Also I'm impressed we're at 4.2 and DnD is this broken still.

The plan is to eventually move to the new Gtk4 listviews based on the new core (#668). I started a branch to port to Gtk3 listboxes because I was getting impatient, but had to stop to fix some bugs and missing stuff in the new core instead.
Also, I was wondering if neui's branch would be a better segway into Gtk4.

If DnD is the only limitation, we could merge this after we plug the new core during the 0.7 cycle. We'll have to accept a temporary regression in master until we complete the switch to listviews.

Marking day as bold isn't needed anymore as GTK has this built in by
default
Make taskeditor mostly work with GTK4.

Notable things to do:
	* Make font selection use css instead of legacy override methods
	* Rethink the recurring menu (fundamentally broken)

Popovers now have a built-in margin, the margins of all the boxes inside
have been reduced by 6px which seems mostly correct.

Setting and getting the position of the window is an archaic X11 concept
not supported in GTK4, that is removed.

the insert_action_group('app', app) and self.insert_action_group('win',
app.browser) don't make sense to me, and those lines broke actions, and
caused cryptic GObject weakref errors. They are simply removed

The popovers are now shown/hidden on window focus out/in as they are now
real windows (when entry is focused). This allows for them to not float
during alt-tab on X11. Note that if the focus is **inside**  the popover
when this happens, then when you return they are still popped down, and
you have to focus the entry twice next time (eventcontroller doesn't
fire for some weird reason), I didn't quite figure that part out.

GtkTextTag::event is gone, and you cannot simply replace it with an
eventcontroller as they are not real widgets.
Instead now the textview itself has a click gesature, and it finds the
tag at the clicked point and calls a function on the tag with convenient
arguments related to clicks.

A bunch of get_iter_at functions have been changed to return a tuple,
instead of a single value. The second value in the tuple is all that is
needed for this usecase.
I separated font_size and font_name into two separate values as that is
easier to represent with CSS.

Also, for some reason GtkFontChooser gives font sizes multiplied by 1000,
so I divided by 1000.

font_size default value is 0 (ignored), I decided to not assume it is 11pt as it
is on GNOME
Now uses a menu model and actions instead of manually constructed
widgets.

I first wanted to use a stateful action for toggling, but I am using a
propertyaction for the checkbutton, and later I realized this actually
seems like quite a good way of doing it.

Now that the RecurringMenu is itself the widget, all the callbacks are
also in that file, which imo makes more sense and is a lot easier to do.
Before this was worked around to not be needed by connecting the signals
after this (16eeb75)

However that isn't really possible with templates, instead simply use
GLib.idle_add which will make it fire after all the currently running
blocking code is complete
We need recent fixes that aren't in a released version that is part of
the runtime yet. (liststore insert_with_valuesv bug)
This is a new liblarch API, this causes the treeview to render with dark
styling
In GTK4 the menubutton css node isn't the button, but the button inside
of it is.

Note that Libadwaita provides an api for such button+linked menu cases
like this which you will need to use in libadwaita
Main change is preferences window is a separate composite template
class (for the signal autoconnect). It does maybe seem slightly weirder
like this.

I tried to preserve the old extremely broken design.

This also used get_children() and storing data in widgets, and getting
data from widgets by traversing their widget hierarchy, which is very
discouraged in GTK4. widget.get_children() -> list(widget) (this is a
pygobject override, the other method requires a ton of calls for
iterating)

There is a warning when disbaling the signal handler for requester
status-changed (with by_func too)
Turns out 2 days ago Tao Zuhong changed this default in GTK commit
9e8bf106538a202ae68450026e3844224ef68221 to be consistent with
GTK3 where it was vertical by default. Explicitly set it to horizontal
I don't see how the hell this worked previously, gobject.set_property
was used for setting them. Now instead use the actual gobject xpad and
ypad properties. (So that they could be customized)
To look like GTK3, note that this is an integer, and the exact correct
amount for both treeviews is a pixel off.

I first actually was setting the ypad on the tagcellrenderer in the task
treeviews, however then a lot later I noticed this actually made the
size inconsistent with tags that don't have any tags. So I switched to
setting it on the text instead, which is consistent and probably better
anyways due to how font size may change.
I didn't figure out the dependencies fully, so I only tested the basic
export actions (although the export actions don't use GTK so it should
be fine). I can confirm they work correctly.

The filechoosernative bug I also encountered while porting another app.
I tried a possibly better workaround of disconnecting the signal handler
on first run however that didn't help.

GtkImage changed to GtkPicture because in GtkImage the pixbuf doesn't
get scaled up fully to take up all the available space anymore.

I opted to not port that to a composite template and instead just
connect signals manually.
For most of these plugins porting to composite templates would be very
weird because the UI is phylosophically "part" of the plugin object itself, but the
plugin object is not a GtkWidget.

The margins at the bottom of the dialog changed by 3 px as I made the
layout a bit more sane (Toplevel box has the margins now). And the fact
that they were inconsistent by 3px was probably an oversight anyway.
The main change here is the migration to GMenu instead of widgets. As
the menu items are immutable, all functions that need to modify one need
a reference to the plugin api (which is different for each task).
So all functions that accept menu items now also need you to provide the
corresponding plugin_api. I also thought about maybe putting the plugin apis
in the menu_items dict.

Also, the prefs.ui file was like 90% useless properties (however
has-entry doesn't make sense there, so I removed it). I didn't replace
builder.connect_signals with a template as there was only one signal.
Adapt to editor being a template
GtkFrame labels are now below the frame, not in it, so it looks a bit
weird. (and not using a custom label widget doesn't change that,
although that wouldn't be possible anyway as we use markup (replace with
css?))

I tried a slightly different approach to compositing preferences this
time, now preferences stores the actual pref dict, and the plugin "gets"
a reference to it.
This time I tried to experiment with a different way of approaching
porting preferences.

Since the prefs look very much like a GtkDialog, and
builder.connect_signals was used only for functionality that is provided
by GtkDialog already, I have ported the window to use a GtkDialog.

I don't understand from where the space between the sentence box and the
vbox2 was comming from in the original (there were no margins or child
spacing set!). So I have simply set an appropriate margin of 20px on the
box.

The buttons look a bit different as I would need to access the box
inside the action_area to make them align to the left.

Also, you do realise using a box with widgets and labels like that doesn't
work with localization, right?

Also, yay: all plugins are ported!
In GTK4 GtkButton has an icon-name property that does this for us. It
also applies the image-button style class which is required for proper
functioning in Libadwaita (without it, it gets treated like a button
with custom content that cannot become flat in toolbars)

Also note that GtkMenuButtons didn't support custom children until very
recently in GTK4, so without this they would require a higher minimum
version.
In GTK4 selecting the currently selected date in a GtkCalendar does not
change the day property or emit the day-selected signal anymore, with no
way to change this behavior.

To still allow selecting the current date as the start date easily, add
a today button.

I don't think this is necessary for the duedade calendar as there is a
"now" button, and how often do you create tasks that are due today
anyway?
The default look is very weird.
By simply setting halign to start, disabling the stupid arrow and
enabling the NESTED mode (which can ONLY be done by using the
new_from_model_full constructor), it looks basically like context
menus usually do.

I think this is a GTK bug/oddity: if you hover over a submenu item to
show the submenu, and then leave it, the submenu doesn't disappear like
it did in GTK3. This also makes you can't open items with a single click
if you go over submenus, seems like an overlooked GTK issue.
Undo support is now in Gtk, not GtkSourceView, and in Gtk the
begin/end not_undoable action have been renamed to
begin/end irreversible action.
The applications-internet icon used by the CalDav backend has been
considered deprecated for many years.

In GNOME 42, this icon has been removed, so you now see an icon not
found image where it is used.

Bundle the icon to continue using it.
I had ignored it in the past, but now it is actually used, so now it is
ported.

I made a small UI change for the main sidebar, as .inline-toolbar has
been removed, and reinventing that ancient pattern would have definetely been
more difficult.

You may also notice some minor changes in the look, as all the
previously used, now removed GtkAlignments used very stupid random
margins instead of using a sane box/spacing layout. So I simply redid
that, and it is a lot simpler to port, and immediately looks a bit nicer.

CellRendererPixbufs now use icon-name instead of that pixbuf from icon
name func, as porting that approach directly would be quite stupid.
Especially as GtkIconTheme gives GtkIconPaintable, and
CellRendererPixbuf needs a GdkTexture. And to convert a GtkIconPaintable
to a GdkTexture efficiently you need to do some manual GskRenderNode and GskRenderer
stuff that is broken in PyGObject.

Instead of getting a random insensitive GtkStyleContext color, now we
simply use the cellrenderer's sensitive property for disabled backends.
@diegogangl
Copy link
Contributor

diegogangl commented Mar 26, 2022

Hey @ranchester2 , could you could open a PR for your liblarch port? Once we get the new core branches in, it would be nice to work on getting this landed. Then we can move on to newcore + list views

@diegogangl
Copy link
Contributor

Closing for now since this had +infinity conflicts, is based on an older version of Gtk and is superceded by the megaport branch

@diegogangl diegogangl closed this Aug 8, 2023
@nekohayo
Copy link
Member

nekohayo commented Aug 8, 2023

For those wishing to help, the successor to this branch is #894

@nekohayo nekohayo added the maintainability Automated tests suite, tooling, refactoring, or anything that makes it easier for developers label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainability Automated tests suite, tooling, refactoring, or anything that makes it easier for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants