Reimplement toolbar with regular boxes and buttons#166
Reimplement toolbar with regular boxes and buttons#166andy128k wants to merge 1 commit intomvo5:masterfrom
Conversation
|
I merged your other PR, can you please rebase ? |
|
@mvo5 ping |
mvo5
left a comment
There was a problem hiding this comment.
This is great, thanks you! Sorry for the long delay. I will merge but one quick question about the primary-toolbar style change
| </packing> | ||
| </child> | ||
| <style> | ||
| <class name="primary-toolbar"/> |
There was a problem hiding this comment.
This might be a silly question(sorry!) but should we keep the primary-toolbar style here? Or is there a deeper reason for removing it?
There was a problem hiding this comment.
Well, no specific reasoning. My goal is to port the app to Gtk4 where this class is absent.
There was a problem hiding this comment.
I've just checked one more time. This class sets -gtk-icon-shadow: none; in Adwaita theme. Which is just a default value. So, there is no visible effect of this class.
| } | ||
| g_list_free(box_children); | ||
|
|
||
| cb(box, image, label); |
There was a problem hiding this comment.
Maybe I'm overthinking it, but should we either here or in the callback ensure that image, label is not a nullptr? But its probably fine as gtk_widget_show() is robust against nullptr so fine I guess :)
There was a problem hiding this comment.
Generally yes, but these buttons are predefined in ui file and all have nested image and label.
GtkToolbar and its family does not exist in Gtk4, hence the change.
This PR is a continuation of #164 and includes those changes too.