-
Notifications
You must be signed in to change notification settings - Fork 12
Replace sugar3 references with sugar4 #9
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
base: main
Are you sure you want to change the base?
Conversation
I've also removed the redefinitions of *ICON_SIZE in sugar4.graphics.icon, we had all that defined in style so duplicating wasn't necessary. I moved Icon from a Gtk.Widget back to a Gtk.Image, I don't see why it was changed before as Gtk.Image still exists and is expected to be the same. There was no need for a Icon.get_gtk_image seeing as I moved back to Gtk.Image. MenuItem has been moved to a Gio.MenuItem, with a Gtk.Box, css styling was applied directly and I think that's not needed as we have a menuitem styling in sugar-artwork. Signed-off-by: Chihurumnaya Ibiam <[email protected]>
MostlyKIGuess
left a comment
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.
MenuItem
The MenuItem class was changed from Gtk.Button to Gio.MenuItem, but as Gio.MenuItem is not a widget, it's a data model class. Henceforth we won't be able to call widget methods.
The following won't work:
- self.add_css_class("menuitem")`
- `self.connect("map", ...)`
- `self.get_sensitive()`
- `self.emit("clicked")`
Ref: https://docs.gtk.org/gtk4/migrating-3to4.html#gtkmenu-gtkmenubar-and-gtkmenuitem-are-gone
We can keep them as Button with our CSS and that should handle the porting, I am not sure if this is even architecturally correct.
Some other nits on menuitem.py:
L:72 self.set_icon(icon.get_gicon())
We don't have get_icon()..
L:87 self._label = Gtk.Label(label=text_label) self.set_label(self._label) # set_label() expects STRING, not widget
Icons
Also all the icon tests were failing and there are mentions of sugar3 in the docstrings.
Does this actually work? Can you override do_snapshot() on Gtk.Image?
I am not able to run any examples/tests for now. But from what I understand:
- Icon has extensive custom rendering logic (SVG loading, color customization, badge rendering)
- It overrides
do_snapshot()anddo_measure()for custom drawing Gtk.Imagewas designed to display pre-rendered content (pixbufs, textures, icon names)- Using
Gtk.Imageas a base but overriding its rendering defeats the purpose defeats our purpose.
We can either keep Widget with custom rendering, original implementation or run the examples/test with Image and see if anything breaks..
- Thank you for removing duplicate Icon Size Constants
Po files
- Thank you for these paths and sugar4 references.
| install: ## Install package in development mode | ||
| pip install -e . | ||
| install: ## Install package | ||
| pip install --root-user-action=ignore . |
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.
Could you explain this? We would prefer development mode for testing right?
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 running this in the live build, and I needed that.
Anyone running in the live build would too.
I haven't tested most of my changes because there's still much that needs to be done to get it to the point where you can see your changes, I've been moving forward by fixing build errors.
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.
We can perhaps have make install-live-build?
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.
That can work.
I do think that the css needs to go, that's why sugar-artwork exists, keeping it is just duplicating what already exists.
I forgot to change those.
I didn't test this like you did, like I said I was going off build errors, I'll test it like you do. You can override
How does this defeat our purpose? Most of the icons in Sugar are installed using sugar-artwork and can be referenced with their icon name when using them, that's exactly why
I'm glad with running the example test with Image, as that'll reduce the complexity of having custom rendering. |
Signed-off-by: Chihurumnaya Ibiam <[email protected]>
I've also removed the redefinitions of *ICON_SIZE in sugar4.graphics.icon, we had all that defined in style so duplicating wasn't necessary.
I moved Icon from a Gtk.Widget back to a Gtk.Image, I don't see why it was changed before as Gtk.Image still exists and is expected to be the same. There was no need for a Icon.get_gtk_image seeing as I moved back to Gtk.Image.
MenuItem has been moved to a Gio.MenuItem, with a Gtk.Box, css styling was applied directly and I think that's not needed as we have a menuitem styling in sugar-artwork.
@MostlyKIGuess kindly review.