-
Notifications
You must be signed in to change notification settings - Fork 354
Changes ToolBarDropDownButton from Gtk.MenuButton to Gtk.Dropdown #2092
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
Merged
+116
−54
Merged
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
05dfbe8
Starts the change of ToolBarDropDownButton to Gtk.Dropdown
pedropaulosuzuki 8458a05
Fix whitespace
pedropaulosuzuki 8816bef
Fix selected property of ListItem and move widgets to ToolBarItem
pedropaulosuzuki 6cebe2e
I'll never get used to { on another line
pedropaulosuzuki d9379b2
whitespace
pedropaulosuzuki 72a32a6
Remove console.log
pedropaulosuzuki 2d5ace6
Only change SelectedIndex if current_index is different from previous…
pedropaulosuzuki fa66ab2
ToolBarDropDownButton - Explicitly override base property SelectedIte…
pedropaulosuzuki 8496c3a
Fix check icon not being visible when selection is zero
pedropaulosuzuki 65f25f7
Fix typo
pedropaulosuzuki 6dba0de
Reduce verbosity with syntactic sugar (omitting type on new)
pedropaulosuzuki 882e504
Turns ToolBarItem into a Gtk.Box
pedropaulosuzuki f8f684e
Remove private Image and Label properties from ToolBarItem
pedropaulosuzuki 7f475e3
Use Gio.ListStore instead of Gtk.StringList
pedropaulosuzuki dbdaf0c
Update CHANGELOG
pedropaulosuzuki f572ab5
Merge branch 'PintaProject:master' into dropdown
pedropaulosuzuki 00dd84a
ToolBarDropDownButton: Separate ToolBarItem from ToolBarItemWidget
pedropaulosuzuki ab99865
Merge branch 'master' into dropdown
pedropaulosuzuki 16e078c
Merge branch 'PintaProject:master' into dropdown
pedropaulosuzuki d3fccf7
Use snake_case for private fields
pedropaulosuzuki e5b0760
Adds ObjectSelect icon
pedropaulosuzuki 363a2f6
Fix typo
pedropaulosuzuki f4de0c3
Move ObjectSelect icon string to Resources.StandardIcons
pedropaulosuzuki f171717
Merge branch 'PintaProject:master' into dropdown
pedropaulosuzuki 41e4ff8
Merge branch 'PintaProject:master' into dropdown
pedropaulosuzuki 28ab4d9
Add comments for ToolBarDropDown implementation details
pedropaulosuzuki e9b4cbc
Revert to using Gtk.StringList instead of Gio.ListStore
pedropaulosuzuki e6494b8
Fix typo
pedropaulosuzuki 04acad7
Merge branch 'PintaProject:master' into dropdown
pedropaulosuzuki f703636
Remove private selected_item field and organize selection update better
pedropaulosuzuki 0c7a8fe
FIx Whitespace
pedropaulosuzuki 8e7bae9
Fix whitespace 2
pedropaulosuzuki 34791d3
Rename SetSelectedIconVisible to SetCheckmarkVisible
pedropaulosuzuki 81aefff
Remove redundante previous_index check
pedropaulosuzuki d721ec8
Fix range comparison
pedropaulosuzuki 4b24aac
Remove unnecessary null check
cameronwhite File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm uneasy about doing non-idiomatic GTK things here like reusing a separate widget.
Could this just create a
ToolBarItemWidget? It's basically the same as the list widgets, except you'd need the option for whether to show the label.A method like
ToolBarItemWidget.Bind(toolbarItem, showLabel, showSelected)could workThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is, it is not a ToolbarItemWidget. It is another widget, where there is no "selected icon", there is, in some cases, no label, and there is only an icon. If we hide those items in one place, they will be hidden everywhere, so we cannot reuse them. So we would need to create another widget just for that, which is not worth the code complexity. It's all local anyway, so this wouldn't be exposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really understanding what you mean here?
My suggestion was that you'd create a separate instance of a ToolbarItemWidget in OnSetupSelectedItem (different than the ones created in OnSetupListItem). So that widget instance can have its options configured appropriately for showing the checkmark / label, and it's a bit more organized than maintaining references to several separate widgets
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but the ToolBarItemWidget has immutable text/icon, we'd need to make it mutable (is it possible, but could conflict with the ToolBarItemWidgets, since the SelectedItem is a single mutable widget and the rest are a bunch of immutable ones). That's why either creating a specific widget for that or doing the widgets as private fields might be a better option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any objections to making the widget mutable - that's actually the intended approach for GTK list views. (e.g. if you have a long list where not all items are visible, the list view might only create the number of widgets required for visible items, and then as you scroll it's just rebinding those widgets to different items in the model - this is the reason for the separate "setup" and "bind" methods in the factory.
But I don't feel too strongly either way since it's conceptually a different type of widget from the list items - it just happens to be similar enough that you could make use of it.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea of having the SelectedItem as a model is for added flexibility, I can think of specific cases where you want to show a different widget depending on which item is selected, maybe with additional buttons and actions for some fancy programs. However, for the general case, you probably want an immutable widget where you just use methods to change the text or an icon. Or, as Alice said, most people don't even go there and just avoid custom factories and use the dropdown as-is. You would never create/show more than one of those widgets at the same time tho, so it's a bit disjoint from the usual ListView logic.
Yeah, it's one of those cases of "we can, but maybe we shouldn't". For example, we would never want a checked item in the SelectedItem, so it would be always there, hidden in the widget tree, taking up memory for something that will never come true. Not that it matters that much, but I think those widgets have different expectations.