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

Shiny new widgets \o/ #405

Merged
merged 7 commits into from
Aug 30, 2023
Merged

Shiny new widgets \o/ #405

merged 7 commits into from
Aug 30, 2023

Conversation

bugaevc
Copy link
Contributor

@bugaevc bugaevc commented Jul 22, 2023

Hi,

this is an attempt to port Tuba to the new libadwaita widgetry (click, click). I'm not an expert in GTK, nor in Tuba's codebase, so I most likely messed something up — please let me know!

This PR only touches the main window (and not even all of its views?), not the compose dialog.

Screenshot from 2023-07-22 22-13-46
Screenshot from 2023-07-22 23-55-08
Screenshot from 2023-07-22 23-55-59
Screenshot from 2023-07-22 23-56-53

@GeopJr
Copy link
Owner

GeopJr commented Jul 23, 2023

Thank you so much for this PR!

I haven't finished reviewing everything yet but I'll start with some comments (there's no rush or expectations!)

src/Dialogs/MainWindow.vala Outdated Show resolved Hide resolved
src/Dialogs/MainWindow.vala Outdated Show resolved Hide resolved
src/Dialogs/ProfileEdit.vala Outdated Show resolved Hide resolved
@bugaevc bugaevc force-pushed the shiny-new-widgets branch 2 times, most recently from b495fec to a8798d0 Compare July 24, 2023 07:59
@bertob
Copy link
Contributor

bertob commented Aug 19, 2023

What's the status here? Would be sweet to have this in the next release, with GNOME 45 coming out next month :)

@bugaevc
Copy link
Contributor Author

bugaevc commented Aug 19, 2023

This is good to go from my side, except I'd probably have to rebase, and use the 45 runtime instead of master. @GeopJr probably needs to do another round of review and testing.

@GeopJr
Copy link
Owner

GeopJr commented Aug 20, 2023

This is not stale but I have postponed it until the cleanup PR (#424) is done!

@oscfdezdz
Copy link
Contributor

oscfdezdz commented Aug 22, 2023

Hi, while using this branch I noticed some deprecation warnings and that the preferences dialog could use other new libadwaita widgets. I've made the changes in this fork and after testing it, it seems that everything works fine although it never hurts to test it more, especially the one that replaces AdwLeaflet in the new account dialog. Feel free to cherry-pick the commits or I can open a PR against this branch if you want 😄

Screenshots

Captura desde 2023-08-22 13-50-30

Captura desde 2023-08-22 13-50-42

Captura desde 2023-08-22 13-50-51

Captura desde 2023-08-22 13-51-03

@GeopJr
Copy link
Owner

GeopJr commented Aug 30, 2023

Main is now somewhat ready, could you rebase @bugaevc?

There have been many changes in that cleanup PR, most importantly for this PR however: All ListBoxes became ListViews. When in a scrolledwindow they have to be in the following structure: GtkScrolledWindow -> AdwClampScrollable -> ListView. No boxes or other children as it will cause issues with the ListView's size.

That might need some changes in anything that inherits from the Base widget

(I've kind-of rushed the cleanup PR a bit since I didn't want to make rebasing even more difficult so there might be bugs!)

If you are busy, let me know and I'll rebase it instead!

bugaevc and others added 7 commits August 30, 2023 19:26
These are the shiny new replacements for AdwLeaflet and AdwFlap.
These messages are very verbose (possibly many messages per second), yet
not interesting to a regular user, they're only for developers debugging
the app. Yet, they were logged to the system journal at the NOTICE
level, which is higher (numerically smaller) than even INFO, and so were
highligheted as important in the journalctl output.

Downgrade the logging to use debug (), which results in the messages not
being logged to the journal at all by default. It is still possible to
see the debugging messages by setting G_MESSAGES_DEBUG=Tuba (or "all").
@bugaevc
Copy link
Contributor Author

bugaevc commented Aug 30, 2023

Yay! Rebased (that took some work) & added @oscfdezdz's changes. Since column_view is gone, I had to improvise again, and this time placed the spinner into the overlay. It still seems to work fine though 🙂

Please review & test!

@bertob
Copy link
Contributor

bertob commented Aug 30, 2023

Just built the branch and gave it a spin, a few things I noticed (not all necessarily coming from this MR, haven't tested the app in a while):

  • The body text in the profile descriptions is bold

image

  • The focus border on posts should have the same shape and consistent padding all around (currently there's extra vertical padding):

image

  • The focus border on boost profile pictures should be circular

image

  • I'd drop the padding between cards on narrow views, and just have simple divider lines between posts:

image

  • Could also be done as a followup, but I'd merge this view switcher into the sidebar at large sizes

image

@GeopJr
Copy link
Owner

GeopJr commented Aug 30, 2023

Yay! Rebased (that took some work) & added [at]oscfdezdz's changes. Since column_view is gone, I had to improvise again, and this time placed the spinner into the overlay. It still seems to work fine though 🙂

Please review & test!

Thanks and sorry about that 🙇

Everything looks good, I'll go ahead and merge it! Thank you so much for making this PR!

Just built the branch and gave it a spin, a few things I noticed (not all necessarily coming from this MR, haven't tested the app in a while):

* The body text in the profile descriptions is bold
* The focus border on posts should have the same shape and consistent padding all around (currently there's extra vertical padding):
* The focus border on boost profile pictures should be circular

Nice catch! I'll get on it on a different PR!

* I'd drop the padding between cards on narrow views, and just have simple divider lines between posts:
* Could also be done as a followup, but I'd merge this view switcher into the sidebar at large sizes

This PR is only the start of #228, as long as they are in the mockup (they are), we'll get there!

@GeopJr GeopJr merged commit d30bfc0 into GeopJr:main Aug 30, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants