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

fix(gnome): No ellipses on Video Format combobox #1050

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

largestgithubuseronearth
Copy link
Contributor

@largestgithubuseronearth largestgithubuseronearth commented Jan 27, 2025

Closes #1047

Summary:

  • Add GtkHelpers::newNoEllipsesItemFactory() which creates a GtkListItemFactory that disables ellipses on every widget in the ListView.
  • This factory has two signal callbacks in helpers/listitemfactory.cpp which do the work of creating the label for each ListItem.
  • Call the above newFactory function inside GtkHelpers::setComboRowModel() and pass the factory into adw_combo_row_set_factory()

This is the result in the GUI
Screenshot From 2025-01-26 18-40-29

This works as expected but is a sloppy C solution for a C++ app structure. There's a few things that need to be fixed:

  • Applies to all ComboRows indiscriminately. Factory needs to reference the GtkStringList to apply the right string to the ListItem label but the GtkStringList only exists inside setComboRowModel().
  • I don't know where to put the factory callbacks since they aren't part of a class. They should be private but they don't feel right in GtkHelpers.
  • The factory constructor has a very narrow use case and an awkwardly specific name. It could be reworked to be more generic and take multiple types of attributes or take closures but that might not be worth the effort if you don't need a ListItemFactory anywhere else.

I'm not very familiar with C++ design so I'd appreciate your opinion on how to reconcile these so it fits neatly into the existing code.

@nlogozzo
Copy link
Member

nlogozzo commented Jan 27, 2025

First off, thank you for this PR! I had no idea this was possible, so thank you again for the implementation!

I committed a more C++ implementation of what you had:

  • The free functions in listitemfactory.cpp were removed and placed inline of g_signal_connect as C++ lambdas
  • Added an allowEllipse parameter to setComboRowModel(), true by default, that when set to false will trigger the construction of the factory and the prevention of ellipses
  • Made the label constructed in the factory setup left aligned instead of center aligned

Let me know if you have any questions on the implementation I committed, I am more than happy to go into more detail!

Thanks again!

@nlogozzo nlogozzo marked this pull request as ready for review January 27, 2025 04:46
@nlogozzo nlogozzo merged commit 2d60e3d into NickvisionApps:main Jan 27, 2025
@largestgithubuseronearth largestgithubuseronearth deleted the fix-popover branch January 28, 2025 04:03
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.

GNOME: AddDownloadDialog: Video formats list is truncated
2 participants