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 Gtk 4 #1896

Merged
merged 133 commits into from
Feb 14, 2024
Merged

Port to Gtk 4 #1896

merged 133 commits into from
Feb 14, 2024

Conversation

danirabbit
Copy link
Member

@danirabbit danirabbit commented Aug 27, 2022

Fixes #1590
Fixes #1795
Fixes #1940

I've tried to comment all non-obvious changes here. A lot of the diff is just minor API changes. Hide whitespace changes might be useful since there's a few try/catches removed or null or type checks added

Categories:

  • css: Fixes for some changes to how Gtk.CSS renders gradients

Application:

  • Use new method for window save/restore size
  • New way to check focus
  • No more destroy, connect to close

Package:

  • It looks like we never copied remote icons before? But now this causes a crash when Gtk tries to set a Paintable from a File. So removed for now since this does nothing already

StripeDialog:

  • Spinbutton no longer has activates_default so use a key controller
  • Spinbutton no longer has primary_icon_name so parent a Gtk.Image

MainWindow:

  • Leaflet handles mouse back button, so no need to do it ourselves

AppInfoView:

  • There was some overcomplicated arrow mouse event stuff here. afaict the motion controller hides and shows arrows more accurately in this branch than master with fewer signals
  • I used Gtk.Picture for screenshots instead of Gtk.Image which seems to obsolete Use DrawingArea for screenshots #1870

AbstractAppContainer:

  • Made overflow visible in a revealer here so that button shadows don't get clipped

@danirabbit danirabbit changed the title Initial build attempt Port to Gtk 4 Aug 27, 2022
@danirabbit danirabbit requested a review from a team February 7, 2024 18:16
@danirabbit danirabbit marked this pull request as ready for review February 7, 2024 18:16
@danirabbit
Copy link
Member Author

@leolost2605 since AppCenter is somewhere you touched recently could I get a review here? I'd like to do some more UI work but I really want to land GTK4 porting first :)

Copy link
Member

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things I noticed during testing, I think most of them aren't blocking though

  • If you've got only a few packages the packages grid in the update view is bottom aligned but I think that's also present in master

  • Also I'm being greeted by a restart required infobar
    image

  • The backbutton doesn't have the back style applied but I've seen the same thing in settings so I think that's a granite/stylesheet issue?

  • Switching between remotes plays an leaflet animation

  • There are a lot of deprecation warnings but I guess those can be fixed in follow up PRs

src/MainWindow.vala Outdated Show resolved Hide resolved
src/Views/CategoryView.vala Outdated Show resolved Hide resolved
@danirabbit
Copy link
Member Author

  • If you've got only a few packages the packages grid in the update view is bottom aligned but I think that's also present in master

Yup that's in master too. Definitely wanna iterate here in a future branch

  • Also I'm being greeted by a restart required infobar

Ah because widgets are visible by default in GTK4 and we forgot to remove this when we removed the updates recently in another branch. Fixed!

  • The backbutton doesn't have the back style applied but I've seen the same thing in settings so I think that's a granite/stylesheet issue?

Yeah, pushed a fix to stylesheet 😅

  • Switching between remotes plays an leaflet animation

Yeah this might be a leaflet problem 🤔 When we move to NavigationView there's a property there for disabling animations, but I was trying to keep from blowing up the diff. Is this okay to fix in a follow up?

  • There are a lot of deprecation warnings but I guess those can be fixed in follow up PRs

Yeah I was trying to keep the diff here as small as I can

Copy link
Member

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good enough to iterate on! 🎉

@danirabbit
Copy link
Member Author

Let's fucking goooo

@danirabbit danirabbit merged commit d541ca3 into master Feb 14, 2024
4 checks passed
@danirabbit danirabbit deleted the gtk4 branch February 14, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
6 participants