search

cvfosammmm / Setzer

LaTeX editor written in Python with Gtk
https://www.cvfosammmm.org/setzer/
Other
388 stars 34 forks source link

Add support to follow system theme #316

Closed oscfdezdz closed 1 year ago

oscfdezdz commented 1 year ago

It adds Libhandy as a dependency, the minimum version required is 1.6 which is not available in distributions like Debian 11. Maybe the dependency could be optional, but before making any other changes I would like to know your opinion about this PR.

Also, there are things that can be improved in commit 5002495e4e1a9acf0c4088576f3c1abe929cb37a, I will try to give it a whirl.

Closes #209 Closes #293

cvfosammmm commented 1 year ago

Thanks a lot for this effort. I think the new requirement is ok.

cvfosammmm commented 1 year ago

Btw. I have converted this PR to draft, since you're still working on it.

cvfosammmm commented 1 year ago

Just let me know when it's ready to merge, will review then.

oscfdezdz commented 1 year ago

It's ready. I'm not convinced about importing libhandy in so many modules, but it's the easiest way I've managed to avoid making major code changes.

cvfosammmm commented 1 year ago

I think the values in settings should be encoded differently, just use strings "force_dark", "force_light", "default". Because we might want to get rid of the Handy requirement in the future.

cvfosammmm commented 1 year ago

I think the preferences page is a bit too tall now, so we should add a ScrolledWindow.

cvfosammmm commented 1 year ago

One more thing: in the view menu, unchecking the darkmode button moves the state to "force_light". I think it should move it to "default".

oscfdezdz commented 1 year ago

Those changes have now been implemented.

oscfdezdz commented 1 year ago

I think the preferences page is a bit too tall now, so we should add a ScrolledWindow.

These changes are now in a different commit.

cvfosammmm commented 1 year ago

Nice. What about my comment about the HeaderBar?

oscfdezdz commented 1 year ago

Nice. What about my comment about the HeaderBar?

One more thing: in the view menu, unchecking the darkmode button moves the state to "force_light". I think it should move it to "default".

Do you mean this one? It's also changed.

cvfosammmm commented 1 year ago

I mean why does the headerbar now depend on Handy?

oscfdezdz commented 1 year ago

I thought it was required when changing ApplicationWindow, but I see it is not. It is reverted now.

oscfdezdz commented 1 year ago

Actually, it seems to be required, I can't move the window around after reverting it.

oscfdezdz commented 1 year ago

From documentation:

HdyApplicationWindow is a GtkApplicationWindow subclass providing the same features as HdyWindow.

It’s recommended to use HdyHeaderBar with HdyWindow, as unlike GtkHeaderBar it remains draggable inside the window. Otherwise, HdyWindowHandle can be used.

Should we use HdyWindowHandle better?

cvfosammmm commented 1 year ago

The headerbar is ok. I was just wondering why you put it in there.

oscfdezdz commented 1 year ago

I'm not sure if I'm following. Although HdyApplicationWindow is a GtkApplicationWindow subclass, set_titlebar method is not supported according to documentation. Checking the features it shares with HdyWindow, one of them being the lack of titlebar and the example provided in its documentation. I put it in a GtkBox with vertical orientation which is required between HdyWindow and its content.

cvfosammmm commented 1 year ago

Nice work, thanks a lot.