From a60eea402ed4c5bd4f7dfe27295f804c330746d1 Mon Sep 17 00:00:00 2001 From: PBS Date: Mon, 25 Mar 2024 21:21:29 +0900 Subject: [PATCH] Rework Popovers and Containers management Avoid situations that require manual unparent()ing, signal_destroy(), and present()ing popovers in size_allocate_vfunc() as much as possible. Prefer more automatic and less fragile solutions. To show a popover over a widget, one of the following patterns is used: * If the widget is a Gtk container, like a Box or a Grid, simply attach it with set_parent(). Nothing else is required. * If the widget is an intrinisically childless Gtk widget, like a Widget or a DrawingArea, call containerize(*this); set_layout_manager(Gtk::BinLayout::create()); in the constructor, and then the rest is the same as above. * For any widget, put it inside a PopoverBin and attach the popover with setPopover(). * In very rare circumstances where none of the above are viable, we have to resort to the manual management we wanted to avoid. Currently this is only done by SpinButton. Eventually upstream is expected to add some version of PopoverBin, as well as make layout managers more ubiquitous. So in the future, either 1+2 or 3 may become the preferred way to deal with popovers. Until it's clear which, we'll use whatever fits best in a given situation. Main changes: * Get rid of menuize.cpp. Popovers now look like menus and have working hover logic all by themselves. The only other thing it did was set_flags(NESTED), which is now done where necessary. * Remove delete_all(), since it now double-deletes. * Remove UI::on_hide_reset(), since it leaked the C instance, and is largely superseded by PopoverBin's single-popover policy. * Add containerize(), use to reimplement Bin/ColorItem/Ruler/PopoverBin. * Shift responsibility of parenting popovers from popover constructors to their callers. * Remove UI::autohide_tooltip - not necessary anymore - tested. Other fixes: * Fix crash in filter editor by avoiding get_color_with_class() in snapshot_vfunc(). * Fix criticals due to Bin not measuring child and under-allocating. * Fix popovers on spinbuttons not appearing, by changing the event propagation phase in on_popup_menu(). * Move set_overflow() call from toolbars.cpp to Bin, since not supposed to call it except on custom widgets. * Ensure GtkCheckButton uses instead of in glade files. Only page-properties.glade had this problem. * ColorPalette: Put menubutton inside a box, and attach the secondary popover to the box rather than to the menubutton. --- share/ui/color-palette.glade | 353 ++++++++++++------------ share/ui/page-properties.glade | 8 +- src/ui/CMakeLists.txt | 24 +- src/ui/containerize.h | 38 +++ src/ui/contextmenu.cpp | 6 +- src/ui/contextmenu.h | 1 + src/ui/dialog/attrdialog.cpp | 9 - src/ui/dialog/attrdialog.h | 1 - src/ui/dialog/color-item.cpp | 64 +++-- src/ui/dialog/color-item.h | 28 +- src/ui/dialog/dialog-multipaned.cpp | 7 - src/ui/dialog/dialog-multipaned.h | 1 - src/ui/dialog/dialog-notebook.cpp | 21 +- src/ui/dialog/dialog-notebook.h | 2 + src/ui/dialog/document-properties.cpp | 23 +- src/ui/dialog/document-properties.h | 4 +- src/ui/dialog/filter-effects-dialog.cpp | 49 ++-- src/ui/dialog/filter-effects-dialog.h | 18 +- src/ui/dialog/livepatheffect-editor.cpp | 9 +- src/ui/dialog/object-attributes.cpp | 2 - src/ui/dialog/objects.cpp | 47 ++-- src/ui/dialog/objects.h | 17 +- src/ui/dialog/svg-fonts-dialog.cpp | 4 +- src/ui/dialog/swatches.cpp | 5 +- src/ui/dialog/xml-tree.cpp | 7 - src/ui/menuize.cpp | 183 ------------ src/ui/menuize.h | 59 ---- src/ui/popup-menu.cpp | 28 +- src/ui/popup-menu.h | 14 +- src/ui/toolbar/page-toolbar.cpp | 1 - src/ui/toolbar/tool-toolbar.cpp | 21 +- src/ui/toolbar/tool-toolbar.h | 2 + src/ui/toolbar/toolbars.cpp | 1 - src/ui/tools/tool-base.cpp | 14 +- src/ui/widget/bin.cpp | 39 ++- src/ui/widget/bin.h | 10 +- src/ui/widget/canvas-grid.cpp | 7 +- src/ui/widget/canvas-grid.h | 12 +- src/ui/widget/color-palette.cpp | 3 +- src/ui/widget/color-palette.h | 3 - src/ui/widget/completion-popup.cpp | 2 +- src/ui/widget/desktop-widget.cpp | 6 - src/ui/widget/desktop-widget.h | 13 +- src/ui/widget/gradient-editor.cpp | 2 +- src/ui/widget/ink-ruler.cpp | 32 +-- src/ui/widget/ink-ruler.h | 7 +- src/ui/widget/page-properties.cpp | 3 +- src/ui/widget/popover-bin.cpp | 33 +++ src/ui/widget/popover-bin.h | 35 +++ src/ui/widget/popover-menu-item.cpp | 3 - src/ui/widget/popover-menu.cpp | 29 +- src/ui/widget/popover-menu.h | 14 +- src/ui/widget/selected-style.cpp | 33 +-- src/ui/widget/selected-style.h | 7 +- src/ui/widget/spinbutton.cpp | 39 ++- src/ui/widget/spinbutton.h | 7 +- src/ui/widget/status-bar.cpp | 8 +- 57 files changed, 606 insertions(+), 812 deletions(-) create mode 100644 src/ui/containerize.h delete mode 100644 src/ui/menuize.cpp delete mode 100644 src/ui/menuize.h create mode 100644 src/ui/widget/popover-bin.cpp create mode 100644 src/ui/widget/popover-bin.h diff --git a/share/ui/color-palette.glade b/share/ui/color-palette.glade index 4b9e31ae5c..b388d131aa 100644 --- a/share/ui/color-palette.glade +++ b/share/ui/color-palette.glade @@ -27,6 +27,7 @@ 2 + start start @@ -169,186 +170,192 @@ - - True - False - True - center - end - 1 - 1 - none - - - - - - - - - start - start - 12 - 12 - 12 - 12 - 7 - 8 - - - start - Tile size: - - 0 - 0 - - - - - - True - 180 - True - adjustment-border - True - 50 - 0 - 0 - right - - 1 - 3 - - - - - - start - Border: - - 0 - 3 - - - - - - True - True - adjustment-size - True - 50 - 0 - 0 - right - - 1 - 0 - - - - - - start - Rows: - - 0 - 4 - - - - - - True - 180 - True - adjustment-rows - True - 50 - 0 - 0 - right - - 1 - 4 - - - - - - start - Aspect: - - 0 - 1 - - - - - - True - 180 - True - adjustment-aspect - True - 50 - right - - 1 - 1 - - - - - - Use scrollbar - True - start - - 1 - 5 - - - + - - Stretch to fill + True - start - - 1 - 2 - + False + True + center + end + 1 + 1 + none + - - Enlarge pinned colors - True - start - - 1 - 6 - - - - - - Show color labels - True - start - True - - 1 - 7 - + + + + start + start + 12 + 12 + 12 + 12 + 7 + 8 + + + start + Tile size: + + 0 + 0 + + + + + + True + 180 + True + adjustment-border + True + 50 + 0 + 0 + right + + 1 + 3 + + + + + + start + Border: + + 0 + 3 + + + + + + True + True + adjustment-size + True + 50 + 0 + 0 + right + + 1 + 0 + + + + + + start + Rows: + + 0 + 4 + + + + + + True + 180 + True + adjustment-rows + True + 50 + 0 + 0 + right + + 1 + 4 + + + + + + start + Aspect: + + 0 + 1 + + + + + + True + 180 + True + adjustment-aspect + True + 50 + right + + 1 + 1 + + + + + + Use scrollbar + True + start + + 1 + 5 + + + + + + Stretch to fill + True + start + + 1 + 2 + + + + + + Enlarge pinned colors + True + start + + 1 + 6 + + + + + + Show color labels + True + start + True + + 1 + 7 + + + + + - + + diff --git a/share/ui/page-properties.glade b/share/ui/page-properties.glade index e09544740e..fa4f60b399 100644 --- a/share/ui/page-properties.glade +++ b/share/ui/page-properties.glade @@ -148,12 +148,12 @@ 5 True page-portrait - + 20 page-landscape-symbolic - + @@ -161,12 +161,12 @@ True 5 True - + 20 page-portrait-symbolic - + diff --git a/src/ui/CMakeLists.txt b/src/ui/CMakeLists.txt index ad12fdbdd0..8ef41a286a 100644 --- a/src/ui/CMakeLists.txt +++ b/src/ui/CMakeLists.txt @@ -11,7 +11,6 @@ set(ui_SRC drag-and-drop.cpp icon-loader.cpp interface.cpp - menuize.cpp monitor.cpp pack.cpp popup-menu.cpp @@ -137,10 +136,9 @@ set(ui_SRC dialog/filter-effects-dialog.cpp dialog/find.cpp dialog/font-collections-manager.cpp - widget/font-collection-selector.cpp dialog/font-substitution.cpp dialog/global-palettes.cpp - dialog/glyphs.cpp + dialog/glyphs.cpp dialog/grid-arrange-tab.cpp dialog/guides.cpp dialog/icon-preview.cpp @@ -200,7 +198,6 @@ set(ui_SRC widget/color-palette-preview.cpp widget/color-picker.cpp widget/color-preview.cpp - icon-loader.cpp widget/color-scales.cpp widget/color-slider.cpp widget/combo-box-entry-tool-item.cpp @@ -208,7 +205,7 @@ set(ui_SRC widget/css-name-class-init.cpp widget/custom-tooltip.cpp widget/dash-selector.cpp - widget/desktop-widget.cpp + widget/desktop-widget.cpp widget/entity-entry.cpp widget/entry.cpp widget/export-lists.cpp @@ -216,6 +213,7 @@ set(ui_SRC widget/filter-effect-chooser.cpp widget/fill-style.cpp widget/font-button.cpp + widget/font-collection-selector.cpp widget/font-selector.cpp widget/font-selector-toolbar.cpp widget/font-variants.cpp @@ -248,6 +246,7 @@ set(ui_SRC widget/paint-selector.cpp widget/pattern-editor.cpp widget/point.cpp + widget/popover-bin.cpp widget/popover-menu.cpp widget/popover-menu-item.cpp widget/preferences-widget.cpp @@ -263,7 +262,7 @@ set(ui_SRC widget/shapeicon.cpp widget/spin-scale.cpp widget/spinbutton.cpp - widget/status-bar.cpp + widget/status-bar.cpp widget/stroke-style.cpp widget/style-subject.cpp widget/style-swatch.cpp @@ -274,7 +273,7 @@ set(ui_SRC widget/unit-menu.cpp widget/unit-tracker.cpp widget/widget-vfuncs-class-init.cpp - widget/xml-treeview.cpp + widget/xml-treeview.cpp view/svg-view-widget.cpp @@ -284,6 +283,7 @@ set(ui_SRC builder-utils.h clipboard.h contextmenu.h + containerize.h controller.h cursor-utils.h control-types.h @@ -295,7 +295,6 @@ set(ui_SRC icon-names.h icon-loader.h interface.h - menuize.h monitor.h pack.h popup-menu.h @@ -481,7 +480,7 @@ set(ui_SRC widget/canvas/glgraphics.h widget/canvas/cairographics.h widget/canvas-grid.h - widget/canvas-notice.h + widget/canvas-notice.h widget/completion-popup.h widget/color-entry.h widget/color-icc-selector.h @@ -498,7 +497,7 @@ set(ui_SRC widget/css-name-class-init.h widget/custom-tooltip.h widget/dash-selector.h - widget/desktop-widget.h + widget/desktop-widget.h widget/entity-entry.h widget/entry.h widget/events/canvas-event.h @@ -541,6 +540,7 @@ set(ui_SRC widget/palette_t.h widget/pattern-editor.h widget/point.h + widget/popover-bin.h widget/popover-menu.h widget/popover-menu-item.h widget/preferences-widget.h @@ -557,7 +557,7 @@ set(ui_SRC widget/shapeicon.h widget/spin-scale.h widget/spinbutton.h - widget/status-bar.h + widget/status-bar.h widget/stroke-style.h widget/style-subject.h widget/style-swatch.h @@ -567,7 +567,7 @@ set(ui_SRC widget/unit-menu.h widget/unit-tracker.h widget/widget-vfuncs-class-init.h - widget/xml-treeview.h + widget/xml-treeview.h view/svg-view-widget.h ) diff --git a/src/ui/containerize.h b/src/ui/containerize.h new file mode 100644 index 0000000000..0a3b4d0210 --- /dev/null +++ b/src/ui/containerize.h @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#ifndef INKSCAPE_UI_CONTAINERIZE_H +#define INKSCAPE_UI_CONTAINERIZE_H + +#include + +namespace Inkscape::UI { + +/** + * Make a custom widget implement sensible memory management for its children. + * + * This frees the implementer of a custom widget from having to manually unparent() + * children added with set_parent() both in the destructor and on signal_destroy(), + * a memory management detail from C that has no business leaking into C++. + * + * Upon destruction, or for managed widgets just before, all children are unparented. + * Managed children are also deleted if they have no other references. + * + * This function is typically called in the constructor of a custom widget that derives + * from an intrinsically childless Gtk widget, e.g. Gtk::Widget, Gtk::DrawingArea. + + * It must not be used with any intrinsically child-containing Gtk widget, e.g. + * Gtk::Box, Gtk::SpinButton. + */ +inline void containerize(Gtk::Widget &widget) +{ + g_signal_connect(widget.gobj(), "destroy", G_CALLBACK(+[] (GtkWidget *gobj, void *) { + for (auto c = gtk_widget_get_first_child(gobj); c; ) { + auto cnext = gtk_widget_get_next_sibling(c); + gtk_widget_unparent(c); + c = cnext; + } + }), nullptr); +} + +} // namespace Inkscape::UI + +#endif // INKSCAPE_UI_CONTAINERIZE_H diff --git a/src/ui/contextmenu.cpp b/src/ui/contextmenu.cpp index 92afb607e8..55d8816b0f 100644 --- a/src/ui/contextmenu.cpp +++ b/src/ui/contextmenu.cpp @@ -47,7 +47,6 @@ #include "object/sp-text.h" #include "object/sp-use.h" #include "ui/desktop/menu-set-tooltips-shift-icons.h" -#include "ui/menuize.h" #include "ui/util.h" #include "ui/widget/desktop-widget.h" @@ -347,14 +346,11 @@ ContextMenu::ContextMenu(SPDesktop *desktop, SPObject *object, bool hide_layers_ } // clang-format on - auto const widget = desktop->getDesktopWidget(); - g_assert(widget); - set_parent(*widget); set_menu_model(gmenu); set_position(Gtk::PositionType::BOTTOM); set_has_arrow(false); show_all_images(*this); - Inkscape::UI::menuize_popover(*this); + set_flags(Gtk::PopoverMenu::Flags::NESTED); // Do not install this CSS provider; it messes up menus with icons (like popup menu with all dialogs). // It doesn't work well with context menu either, introducing disturbing visual glitch diff --git a/src/ui/contextmenu.h b/src/ui/contextmenu.h index a66f8c8244..bdd23861f4 100644 --- a/src/ui/contextmenu.h +++ b/src/ui/contextmenu.h @@ -40,6 +40,7 @@ private: std::vector items_under_cursor; void unhide_or_unlock(SPDocument* document, bool unhide); }; + #endif // SEEN_CONTEXT_MENU_H /* diff --git a/src/ui/dialog/attrdialog.cpp b/src/ui/dialog/attrdialog.cpp index dd157f71f3..987e636abf 100644 --- a/src/ui/dialog/attrdialog.cpp +++ b/src/ui/dialog/attrdialog.cpp @@ -57,7 +57,6 @@ #include "ui/dialog/inkscape-preferences.h" #include "ui/icon-loader.h" #include "ui/icon-names.h" -#include "ui/menuize.h" #include "ui/pack.h" #include "ui/popup-menu.h" #include "ui/syntax.h" @@ -235,7 +234,6 @@ AttrDialog::AttrDialog() action->property_state().signal_changed().connect([=, this]{ int n; action->get_state(n); setPrecision(n); }); insert_action_group("attrdialog", std::move(group)); - UI::menuize_popover(*get_widget(_builder, "btn-menu").get_popover()); attr_reset_context(0); UI::pack_start(*this, get_widget(_builder, "main-box"), UI::PackOptions::expand_widget); @@ -281,13 +279,6 @@ void AttrDialog::adjust_popup_edit_size() } } -void AttrDialog::size_allocate_vfunc(int const width, int const height, int const baseline) -{ - DialogBase::size_allocate_vfunc(width, height, baseline); - - _popover->present(); -} - bool AttrDialog::onPopoverKeyPressed(GtkEventControllerKey const * /*controller*/, unsigned keyval, unsigned /*keycode*/, GdkModifierType state) diff --git a/src/ui/dialog/attrdialog.h b/src/ui/dialog/attrdialog.h index f586070b8e..d16c2d9344 100644 --- a/src/ui/dialog/attrdialog.h +++ b/src/ui/dialog/attrdialog.h @@ -154,7 +154,6 @@ private: auto_connection _close_popup; int _rounding_precision = 0; - void size_allocate_vfunc(int width, int height, int baseline) final; bool onPopoverKeyPressed(GtkEventControllerKey const *controller, unsigned keyval, unsigned keycode, GdkModifierType state); diff --git a/src/ui/dialog/color-item.cpp b/src/ui/dialog/color-item.cpp index a2fcbe7e5a..42de70f29c 100644 --- a/src/ui/dialog/color-item.cpp +++ b/src/ui/dialog/color-item.cpp @@ -28,16 +28,17 @@ #include #include #include +#include #include #include #include +#include #include #include "desktop-style.h" #include "document.h" #include "document-undo.h" #include "hsluv.h" -#include "inkscape-preferences.h" #include "message-context.h" #include "preferences.h" #include "selection.h" @@ -45,34 +46,41 @@ #include "display/cairo-utils.h" #include "helper/sigc-track-obj.h" #include "io/resource.h" -#include "io/sys.h" #include "object/sp-gradient.h" #include "object/tags.h" #include "svg/svg-color.h" +#include "ui/containerize.h" #include "ui/controller.h" #include "ui/dialog/dialog-base.h" #include "ui/dialog/dialog-container.h" #include "ui/icon-names.h" -#include "ui/menuize.h" #include "ui/util.h" -[[nodiscard]] static auto const &get_removecolor() +namespace Inkscape::UI::Dialog { +namespace { + +// Return the result of executing a lambda, and cache the result for future calls. +template +auto &staticify(F &&f) { - // The "remove-color" image. - static Glib::RefPtr pixbuf; - if (pixbuf) return pixbuf; - - auto path_utf8 = (Glib::ustring)Inkscape::IO::Resource::get_path(Inkscape::IO::Resource::SYSTEM, - Inkscape::IO::Resource::UIS, "resources", "remove-color.png"); - auto path = Glib::filename_from_utf8(path_utf8); - pixbuf = Gdk::Pixbuf::create_from_file(path); - if (!pixbuf) { - g_warning("Null pixbuf for %p [%s]", path.c_str(), path.c_str()); - } - return pixbuf; + static auto result = std::forward(f)(); + return result; } -namespace Inkscape::UI::Dialog { +// Get the "remove-color" image. +Glib::RefPtr get_removecolor() +{ + return staticify([] { + auto path = IO::Resource::get_path(IO::Resource::SYSTEM, IO::Resource::UIS, "resources", "remove-color.png"); + auto pixbuf = Gdk::Pixbuf::create_from_file(path.pointer()); + if (!pixbuf) { + std::cerr << "Null pixbuf for " << Glib::filename_to_utf8(path.pointer()) << std::endl; + } + return pixbuf; + }); +} + +} // namespace ColorItem::ColorItem(PaintDef const &paintdef, DialogBase *dialog) : dialog(dialog) @@ -120,7 +128,9 @@ ColorItem::ColorItem(SPGradient *gradient, DialogBase *dialog) common_setup(); } -ColorItem::ColorItem(Glib::ustring name) : description(std::move(name)) { +ColorItem::ColorItem(Glib::ustring name) + : description(std::move(name)) +{ bool group = !description.empty(); set_name("ColorItem"); set_tooltip_text(description); @@ -140,8 +150,11 @@ bool ColorItem::is_filler() const { void ColorItem::common_setup() { + containerize(*this); + set_layout_manager(Gtk::BinLayout::create()); set_name("ColorItem"); set_tooltip_text(description + (tooltip.empty() ? tooltip : "\n" + tooltip)); + set_draw_func(sigc::mem_fun(*this, &ColorItem::draw_func)); Controller::add_drag_source(*this, { @@ -267,7 +280,7 @@ void ColorItem::draw_func(Cairo::RefPtr const &cr, int const w, } } -void ColorItem::size_allocate_vfunc(int const width, int const height, int const baseline) +void ColorItem::size_allocate_vfunc(int width, int height, int baseline) { Gtk::DrawingArea::size_allocate_vfunc(width, height, baseline); @@ -413,10 +426,15 @@ void ColorItem::on_rightclick() menu->append_section(section); } - // static to only create/show 1 menu over all items & avoid lifetime hassles - static std::unique_ptr popover; - popover = UI::make_menuized_popover(std::move(menu), *this); - popover->popup(); + + if (_popover) { + _popover->unparent(); + } + + _popover = std::make_unique(menu, Gtk::PopoverMenu::Flags::NESTED); + _popover->set_parent(*this); + + _popover->popup(); } void ColorItem::action_set_fill() diff --git a/src/ui/dialog/color-item.h b/src/ui/dialog/color-item.h index b06de94bad..61d1078c54 100644 --- a/src/ui/dialog/color-item.h +++ b/src/ui/dialog/color-item.h @@ -34,6 +34,7 @@ class Drag; namespace Gtk { class DragSource; class GestureClick; +class Popover; } // namespace Gtk class SPGradient; @@ -47,18 +48,18 @@ class DialogBase; * * Note: This widget must be outlived by its parent dialog, passed in the constructor. */ -class ColorItem final : public Gtk::DrawingArea +class ColorItem : public Gtk::DrawingArea { public: /// Create a static color from a paintdef. ColorItem(PaintDef const&, DialogBase*); /// Add new group or filler element. ColorItem(Glib::ustring name); - ~ColorItem() final; + ~ColorItem() override; // Returns true if this is group heading rather than a color bool is_group() const; - // Returns true if this is alignmet filler item, not a color + // Returns true if this is alignment filler item, not a color bool is_filler() const; // Is paint "None"? bool is_paint_none() const; @@ -86,23 +87,19 @@ public: private: void draw_func(Cairo::RefPtr const&, int width, int height); - void size_allocate_vfunc(int width, int height, int baseline) final; + void size_allocate_vfunc(int width, int height, int baseline) override; - Glib::RefPtr on_drag_prepare(Gtk::DragSource const &source, - double x, double y); + Glib::RefPtr on_drag_prepare(Gtk::DragSource const &source, double x, double y); void on_drag_begin(Gtk::DragSource &source, Glib::RefPtr const &drag); // Common post-construction setup. void common_setup(); - void on_motion_enter(GtkEventControllerMotion const *motion, - double x, double y); + void on_motion_enter(GtkEventControllerMotion const *motion, double x, double y); void on_motion_leave(GtkEventControllerMotion const *motion); - Gtk::EventSequenceState on_click_pressed (Gtk::GestureClick const &click, - int n_press, double x, double y); - Gtk::EventSequenceState on_click_released(Gtk::GestureClick const &click, - int n_press, double x, double y); + Gtk::EventSequenceState on_click_pressed (Gtk::GestureClick const &click, int n_press, double x, double y); + Gtk::EventSequenceState on_click_released(Gtk::GestureClick const &click, int n_press, double x, double y); // Perform the on-click action of setting the fill or stroke. void on_click(bool stroke); @@ -137,10 +134,10 @@ private: bool pinned_default = false; // The color. + struct Undefined {}; + struct PaintNone {}; struct RGBData { std::array rgb; }; struct GradientData { SPGradient *gradient; }; - enum Undefined {}; - enum PaintNone {}; std::variant data; // The dialog this widget belongs to. Used for determining what desktop to take action on. @@ -153,6 +150,7 @@ private: // A cache of the widget contents, if necessary. Cairo::RefPtr cache; bool cache_dirty = true; + bool was_grad_pinned = false; // For ensuring that clicks that release outside the widget don't count. @@ -160,6 +158,8 @@ private: sigc::signal _signal_modified; sigc::signal _signal_pinned; + + std::unique_ptr _popover; }; } // namespace Inkscape::UI::Dialog diff --git a/src/ui/dialog/dialog-multipaned.cpp b/src/ui/dialog/dialog-multipaned.cpp index 22379cd1c0..f9061af73a 100644 --- a/src/ui/dialog/dialog-multipaned.cpp +++ b/src/ui/dialog/dialog-multipaned.cpp @@ -490,16 +490,9 @@ DialogMultipaned::DialogMultipaned(Gtk::Orientation orientation) // add empty widget to initiate the container add_empty_widget(); - - signal_destroy().connect([this]{ unparent_children(); }); } DialogMultipaned::~DialogMultipaned() -{ - unparent_children(); -} - -void DialogMultipaned::unparent_children() { // Remove widgets that require special logic to remove. // TODO: Understand why this is necessary. diff --git a/src/ui/dialog/dialog-multipaned.h b/src/ui/dialog/dialog-multipaned.h index 9bc59f26b2..618800cf75 100644 --- a/src/ui/dialog/dialog-multipaned.h +++ b/src/ui/dialog/dialog-multipaned.h @@ -118,7 +118,6 @@ private: // Others Gtk::Widget *_empty_widget; // placeholder in an empty container - void unparent_children(); void insert(int pos, std::unique_ptr child); void add_empty_widget(); void remove_empty_widget(); diff --git a/src/ui/dialog/dialog-notebook.cpp b/src/ui/dialog/dialog-notebook.cpp index f704bf9722..4bd3da2290 100644 --- a/src/ui/dialog/dialog-notebook.cpp +++ b/src/ui/dialog/dialog-notebook.cpp @@ -54,8 +54,8 @@ std::list DialogNotebook::_instances; DialogNotebook::DialogNotebook(DialogContainer *container) : Gtk::ScrolledWindow() , _container(container) - , _menu {nullptr, Gtk::PositionType::BOTTOM} - , _menutabs{*this , Gtk::PositionType::BOTTOM} + , _menu{Gtk::PositionType::BOTTOM} + , _menutabs{Gtk::PositionType::BOTTOM} , _labels_auto(true) , _detaching_duplicate(false) , _selected_page(nullptr) @@ -145,8 +145,7 @@ DialogNotebook::DialogNotebook(DialogContainer *container) return a.order < b.order; }); - auto builder = ColumnMenuBuilder{_menu, 2, Gtk::IconSize::NORMAL, - row}; + auto builder = ColumnMenuBuilder{_menu, 2, Gtk::IconSize::NORMAL, row}; for (auto const &data : all_dialogs) { auto callback = [key = data.key]{ // get desktop's container, it may be different than current '_container'! @@ -156,8 +155,7 @@ DialogNotebook::DialogNotebook(DialogContainer *container) } } }; - builder.add_item(data.label, data.category, {}, data.icon_name, true, false, - std::move(callback)); + builder.add_item(data.label, data.category, {}, data.icon_name, true, false, std::move(callback)); if (builder.new_section()) { builder.set_section(gettext(dialog_categories[data.category])); } @@ -188,8 +186,9 @@ DialogNotebook::DialogNotebook(DialogContainer *container) // ============= Finish setup =============== _reload_context = true; - set_child(_notebook); - set_visible(true); + _popoverbin.setChild(&_notebook); + _popoverbin.setPopover(&_menutabs); + set_child(_popoverbin); _instances.push_back(this); } @@ -209,8 +208,6 @@ DialogNotebook::~DialogNotebook() } _instances.remove(this); - - _menutabs.unparent(); } void DialogNotebook::add_highlight_header() @@ -589,7 +586,7 @@ void DialogNotebook::on_size_allocate_scroll(int const width) // magic number static constexpr int MIN_HEIGHT = 60; // set or unset scrollbars to completely hide a notebook - // because we have a "blocking" scroll per tab we need to loop to aboid + // because we have a "blocking" scroll per tab we need to loop to avoid // other page stop out scroll for_each_page(_notebook, [this](Gtk::Widget &page){ if (!provide_scroll(page)) { @@ -772,7 +769,7 @@ void DialogNotebook::reload_tab_menu() _connmenu.clear(); - _menutabs.delete_all(); + _menutabs.remove_all(); auto prefs = Inkscape::Preferences::get(); bool symbolic = false; diff --git a/src/ui/dialog/dialog-notebook.h b/src/ui/dialog/dialog-notebook.h index 8ab3d0a8aa..54881b73bf 100644 --- a/src/ui/dialog/dialog-notebook.h +++ b/src/ui/dialog/dialog-notebook.h @@ -25,6 +25,7 @@ #include "helper/auto-connection.h" #include "ui/widget/popover-menu.h" +#include "ui/widget/popover-bin.h" namespace Glib { class ValueBase; @@ -85,6 +86,7 @@ private: UI::Widget::PopoverMenu _menu; UI::Widget::PopoverMenu _menutabs; Gtk::Notebook _notebook; + UI::Widget::PopoverBin _popoverbin; // State variables bool _label_visible; diff --git a/src/ui/dialog/document-properties.cpp b/src/ui/dialog/document-properties.cpp index c4a4a77080..44cb877a0b 100644 --- a/src/ui/dialog/document-properties.cpp +++ b/src/ui/dialog/document-properties.cpp @@ -34,7 +34,7 @@ #include #include #include -#include + #include #include #include @@ -143,7 +143,7 @@ static void docprops_style_button(Gtk::Button& btn, char const* iconName) } static bool do_remove_popup_menu(PopupMenuOptionalClick const click, - Gtk::TreeView &tree_view, sigc::slot const &slot) + Gtk::TreeView &tree_view, UI::Widget::PopoverBin &pb, sigc::slot const &slot) { auto const selection = tree_view.get_selection(); if (!selection) return false; @@ -153,9 +153,10 @@ static bool do_remove_popup_menu(PopupMenuOptionalClick const click, auto const mi = Gtk::make_managed(_("_Remove"), true); mi->signal_activate().connect(slot); - auto const menu = std::make_shared(tree_view, Gtk::PositionType::BOTTOM); + auto const menu = Gtk::make_managed(Gtk::PositionType::BOTTOM); menu->append(*mi); - UI::on_hide_reset(menu); + + pb.setPopover(menu); if (click) { menu->popup_at(tree_view, click->x, click->y); @@ -171,10 +172,9 @@ static bool do_remove_popup_menu(PopupMenuOptionalClick const click, return true; } -static void connect_remove_popup_menu(Gtk::TreeView &tree_view, sigc::slot slot) +static void connect_remove_popup_menu(Gtk::TreeView &tree_view, UI::Widget::PopoverBin &pb, sigc::slot slot) { - UI::on_popup_menu(tree_view, sigc::bind(&do_remove_popup_menu, - std::ref(tree_view), std::move(slot))); + UI::on_popup_menu(tree_view, sigc::bind(&do_remove_popup_menu, std::ref(tree_view), std::ref(pb), std::move(slot))); } DocumentProperties::DocumentProperties() @@ -206,7 +206,8 @@ DocumentProperties::DocumentProperties() , _namedview_connection(this) , _root_connection(this) { - UI::pack_start(*this, _notebook, true, true); + append(_popoverbin); + _popoverbin.setChild(&_notebook); _notebook.append_page(*_page_page, _("Display")); _notebook.append_page(*_page_guides, _("Guides")); @@ -878,7 +879,7 @@ void DocumentProperties::build_cms() _LinkedProfilesList.get_selection()->signal_changed().connect( sigc::mem_fun(*this, &DocumentProperties::onColorProfileSelectRow) ); - connect_remove_popup_menu(_LinkedProfilesList, sigc::mem_fun(*this, &DocumentProperties::removeSelectedProfile)); + connect_remove_popup_menu(_LinkedProfilesList, _popoverbin, sigc::mem_fun(*this, &DocumentProperties::removeSelectedProfile)); if (auto document = getDocument()) { std::vector current = document->getResourceList( "defs" ); @@ -1049,8 +1050,8 @@ void DocumentProperties::build_scripting() _external_remove_btn.signal_clicked().connect(sigc::mem_fun(*this, &DocumentProperties::removeExternalScript)); _embed_remove_btn.signal_clicked().connect(sigc::mem_fun(*this, &DocumentProperties::removeEmbeddedScript)); - connect_remove_popup_menu(_ExternalScriptsList, sigc::mem_fun(*this, &DocumentProperties::removeExternalScript)); - connect_remove_popup_menu(_EmbeddedScriptsList, sigc::mem_fun(*this, &DocumentProperties::removeEmbeddedScript)); + connect_remove_popup_menu(_ExternalScriptsList, _popoverbin, sigc::mem_fun(*this, &DocumentProperties::removeExternalScript)); + connect_remove_popup_menu(_EmbeddedScriptsList, _popoverbin, sigc::mem_fun(*this, &DocumentProperties::removeEmbeddedScript)); //TODO: review this observers code: if (auto document = getDocument()) { diff --git a/src/ui/dialog/document-properties.h b/src/ui/dialog/document-properties.h index cccecb382a..e947572ed1 100644 --- a/src/ui/dialog/document-properties.h +++ b/src/ui/dialog/document-properties.h @@ -42,6 +42,7 @@ #include "object/sp-grid.h" #include "ui/dialog/dialog-base.h" +#include "ui/widget/popover-bin.h" #include "ui/widget/licensor.h" #include "ui/widget/registered-widget.h" #include "ui/widget/registry.h" @@ -129,7 +130,8 @@ protected: void set_viewbox_size(SPDesktop* desktop, double width, double height); Inkscape::XML::SignalObserver _emb_profiles_observer, _scripts_observer; - Gtk::Notebook _notebook; + UI::Widget::PopoverBin _popoverbin; + Gtk::Notebook _notebook; UI::Widget::NotebookPage *_page_page; UI::Widget::NotebookPage *_page_guides; diff --git a/src/ui/dialog/filter-effects-dialog.cpp b/src/ui/dialog/filter-effects-dialog.cpp index 021a37cb77..0129b5c5f5 100644 --- a/src/ui/dialog/filter-effects-dialog.cpp +++ b/src/ui/dialog/filter-effects-dialog.cpp @@ -17,7 +17,6 @@ #include "filter-effects-dialog.h" -#include #include #include #include @@ -103,6 +102,8 @@ using Inkscape::UI::Widget::SpinScale; constexpr int max_convolution_kernel_size = 10; +static Glib::ustring const prefs_path = "/dialogs/filters"; + // Returns the number of inputs available for the filter primitive type static int input_count(const SPFilterPrimitive* prim) { @@ -1301,7 +1302,7 @@ static std::unique_ptr create_popup_menu(Gtk::Widget &p sigc::slot dup, sigc::slot rem) { - auto menu = std::make_unique(parent, Gtk::PositionType::RIGHT); + auto menu = std::make_unique(Gtk::PositionType::RIGHT); auto mi = Gtk::make_managed(_("_Duplicate"), true); mi->signal_activate().connect(std::move(dup)); @@ -1413,7 +1414,7 @@ void FilterEffectsDialog::FilterModifier::update_selection(Selection *sel) std::unique_ptr FilterEffectsDialog::FilterModifier::create_menu() { - auto menu = std::make_unique(*this, Gtk::PositionType::BOTTOM); + auto menu = std::make_unique(Gtk::PositionType::BOTTOM); auto append = [&](Glib::ustring const &text, auto const mem_fun) { auto &item = *Gtk::make_managed(text, true); @@ -1604,6 +1605,7 @@ FilterEffectsDialog::FilterModifier::filter_list_click_released(Gtk::GestureClic items.at(0)->set_sensitive(sensitive); items.at(1)->set_sensitive(sensitive); items.at(3)->set_sensitive(sensitive); + _dialog._popoverbin.setPopover(_menu.get()); _menu->popup_at(_list, x, y); return Gtk::EventSequenceState::CLAIMED; } @@ -1761,9 +1763,12 @@ void FilterEffectsDialog::CellRendererConnection::get_preferred_height_for_width /*** PrimitiveList ***/ FilterEffectsDialog::PrimitiveList::PrimitiveList(FilterEffectsDialog& d) - : _dialog(d), - _in_drag(0), - _observer(std::make_unique()) + : Glib::ObjectBase{"FilterEffectsDialogPrimitiveList"} + , WidgetVfuncsClassInit{} + , Gtk::TreeView{} + , _dialog(d) + , _in_drag(0) + , _observer(std::make_unique()) { _inputs_count = FPInputConverter._length; @@ -1797,7 +1802,12 @@ FilterEffectsDialog::PrimitiveList::PrimitiveList(FilterEffectsDialog& d) int cols_count = append_column(_("Connections"), _connection_cell); Gtk::TreeViewColumn* col = get_column(cols_count - 1); if(col) - col->add_attribute(_connection_cell.property_primitive(), _columns.primitive); + col->add_attribute(_connection_cell.property_primitive(), _columns.primitive); +} + +void FilterEffectsDialog::PrimitiveList::css_changed(GtkCssStyleChange *) +{ + bg_color = get_color_with_class(*this, "theme_bg_color"); } // Sets up a vertical Pango context/layout, and returns the largest @@ -1894,11 +1904,9 @@ void FilterEffectsDialog::PrimitiveList::update() } } -void FilterEffectsDialog::PrimitiveList::set_menu(Gtk::Widget &parent, - sigc::slot dup, - sigc::slot rem) +void FilterEffectsDialog::PrimitiveList::set_menu(sigc::slot dup, sigc::slot rem) { - _primitive_menu = create_popup_menu(parent, std::move(dup), std::move(rem)); + _primitive_menu = create_popup_menu(_dialog, std::move(dup), std::move(rem)); } SPFilterPrimitive* FilterEffectsDialog::PrimitiveList::get_selected() @@ -1956,7 +1964,6 @@ void FilterEffectsDialog::PrimitiveList::snapshot_vfunc(Glib::RefPtrtranslate(x_origin, y_origin); auto const fg_color = get_color(); - auto const bg_color = get_color_with_class(*this, "theme_bg_color"); auto bar_color = mix_colors(bg_color, fg_color, 0.06); // color of connector arrow heads and effect separator lines auto mid_color = mix_colors(bg_color, fg_color, 0.16); @@ -2488,6 +2495,7 @@ FilterEffectsDialog::PrimitiveList::on_click_released(Gtk::GestureClick const &c if (click.get_current_button() == 3) { bool const sensitive = prim != nullptr; _primitive_menu->set_sensitive(sensitive); + _dialog._popoverbin.setPopover(_primitive_menu.get()); _primitive_menu->popup_at(*this, wx + 4, wy); return Gtk::EventSequenceState::CLAIMED; } @@ -2712,7 +2720,7 @@ void FilterEffectsDialog::add_effects(Inkscape::UI::Widget::CompletionPopup& pop effect.icon_name, true, true, [=, this]{ add_filter_primitive(type); }); auto const id = static_cast(type); - menuitem->signal_query_tooltip().connect([=](int x, int y, bool kbd, const Glib::RefPtr& tooltipw){ + menuitem->signal_query_tooltip().connect([=, this] (int x, int y, bool kbd, const Glib::RefPtr& tooltipw) { return sp_query_custom_tooltip(this, x, y, kbd, tooltipw, id, effect.tooltip, effect.icon_name); }, false); // before if (builder.new_section()) { @@ -2836,14 +2844,14 @@ FilterEffectsDialog::FilterEffectsDialog() }); _add_primitive.signal_clicked().connect(sigc::mem_fun(*this, &FilterEffectsDialog::add_primitive)); - _primitive_list.set_menu(*this, - sigc::mem_fun(*this, &FilterEffectsDialog::duplicate_primitive), + _primitive_list.set_menu(sigc::mem_fun(*this, &FilterEffectsDialog::duplicate_primitive), sigc::mem_fun(_primitive_list, &PrimitiveList::remove_selected)); get_widget(_builder, "new-filter").signal_clicked().connect([this]{ _filter_modifier.add_filter(); }); append(_bin); - _bin.set_child(_main_grid); _bin.set_expand(true); + _bin.set_child(_popoverbin); + _popoverbin.setChild(&_main_grid); get_widget(_builder, "dup-btn").signal_clicked().connect([this]{ duplicate_primitive(); }); get_widget(_builder, "del-btn").signal_clicked().connect([this]{ _primitive_list.remove_selected(); }); @@ -2856,18 +2864,18 @@ FilterEffectsDialog::FilterEffectsDialog() // full rebuild: this is what it takes to make cell renderer new min width into account to adjust scrollbar _primitive_list.update(); }; - auto show_all_sources = Inkscape::Preferences::get()->getBool(_prefs + "/dialogs/filters/showAllSources", false); + auto show_all_sources = Inkscape::Preferences::get()->getBool(prefs_path + "/dialogs/filters/showAllSources", false); _show_sources->set_active(show_all_sources); set_inputs(show_all_sources); _show_sources->signal_toggled().connect([=, this]{ bool const show_all = _show_sources->get_active(); set_inputs(show_all); - Inkscape::Preferences::get()->setBool(_prefs + "/dialogs/filters/showAllSources", show_all); + Inkscape::Preferences::get()->setBool(prefs_path + "/dialogs/filters/showAllSources", show_all); }); - _paned.set_position(Inkscape::Preferences::get()->getIntLimited(_prefs + "/handlePos", 200, 10, 9999)); + _paned.set_position(Inkscape::Preferences::get()->getIntLimited(prefs_path + "/handlePos", 200, 10, 9999)); _paned.property_position().signal_changed().connect([this]{ - Inkscape::Preferences::get()->setInt(_prefs + "/handlePos", _paned.get_position()); + Inkscape::Preferences::get()->setInt(prefs_path + "/handlePos", _paned.get_position()); }); _primitive_list.update(); @@ -3287,7 +3295,6 @@ void FilterEffectsDialog::update_automatic_region(Gtk::CheckButton *btn) bool automatic = btn->get_active(); _region_pos->set_sensitive(!automatic); _region_size->set_sensitive(!automatic); - } } // namespace Inkscape::UI::Dialog diff --git a/src/ui/dialog/filter-effects-dialog.h b/src/ui/dialog/filter-effects-dialog.h index 49ab67041f..94bad3ad01 100644 --- a/src/ui/dialog/filter-effects-dialog.h +++ b/src/ui/dialog/filter-effects-dialog.h @@ -44,6 +44,8 @@ #include "ui/widget/bin.h" #include "ui/widget/combo-enums.h" #include "ui/widget/completion-popup.h" +#include "ui/widget/popover-bin.h" +#include "ui/widget/widget-vfuncs-class-init.h" #include "xml/helper-observer.h" namespace Gdk { @@ -218,7 +220,9 @@ private: Glib::Property _primitive; }; - class PrimitiveList : public Gtk::TreeView + class PrimitiveList + : public UI::Widget::WidgetVfuncsClassInit + , public Gtk::TreeView { using parent_type = Gtk::TreeView; @@ -228,9 +232,7 @@ private: sigc::signal& signal_primitive_changed(); void update(); - void set_menu(Gtk::Widget &parent, - sigc::slot dup, - sigc::slot rem); + void set_menu(sigc::slot dup, sigc::slot rem); SPFilterPrimitive* get_selected(); void select(SPFilterPrimitive *prim); @@ -241,7 +243,8 @@ private: int get_inputs_count() const; private: - void snapshot_vfunc(Glib::RefPtr const &snapshot) final; + void snapshot_vfunc(Glib::RefPtr const &snapshot) override; + void css_changed(GtkCssStyleChange *change) override; void on_drag_end(Gtk::DragSource const &source, Glib::RefPtr const &drag, bool delete_data); @@ -284,7 +287,8 @@ private: std::unique_ptr _observer; int _input_type_width {}; int _input_type_height{}; - int _inputs_count {}; + int _inputs_count{}; + Gdk::RGBA bg_color{}; }; void init_settings_widgets(); @@ -311,7 +315,7 @@ private: Glib::RefPtr _builder; UI::Widget::Bin _bin; - Glib::ustring _prefs = "/dialogs/filters"; + UI::Widget::PopoverBin _popoverbin; Gtk::Paned& _paned; Gtk::Grid& _main_grid; Gtk::Box& _params_box; diff --git a/src/ui/dialog/livepatheffect-editor.cpp b/src/ui/dialog/livepatheffect-editor.cpp index b30274e70a..5d7678a174 100644 --- a/src/ui/dialog/livepatheffect-editor.cpp +++ b/src/ui/dialog/livepatheffect-editor.cpp @@ -60,7 +60,6 @@ #include "ui/controller.h" #include "ui/icon-loader.h" #include "ui/icon-names.h" -#include "ui/menuize.h" #include "ui/pack.h" #include "ui/tools/node-tool.h" #include "ui/util.h" @@ -331,11 +330,11 @@ void LivePathEffectEditor::add_lpes(Inkscape::UI::Widget::CompletionPopup &popup // 3-column menu // Due to when we rebuild, itʼs not so easy to only populate when the MenuButton is clicked, so - // we remove existing children. We also want to free them, BUT theyʼre Gtk::managed()d, so… h4x - // TODO: GTK4: Use MenuButton.set_create_popup_func() to create new menu every time, on demand? + // we remove existing children. + // TODO: Use MenuButton.set_create_popup_func() to create new menu every time, on demand? auto &menu = popup.get_menu(); - menu.delete_all(); + menu.remove_all(); ColumnMenuBuilder builder{menu, 3, Gtk::IconSize::LARGE}; auto const tie = [](LPEMetadata const &lpe){ return std::tie(lpe.category, lpe.label); }; @@ -795,8 +794,6 @@ LivePathEffectEditor::effect_list_reload(SPLPEItem *lpeitem) // Add actions used by LPEEffectMenuButton add_item_actions(lperef, untranslated_label, *LPEEffect, counter == 0, counter == total - 1); - // & make it act more like ye olde GtkMenu - UI::menuize_popover(*get_widget(builder, "LPEEffectMenuButton").get_popover()); if (total > 1) { auto &source = Controller::add_drag_source(*LPEDrag, {.actions = Gdk::DragAction::MOVE}); diff --git a/src/ui/dialog/object-attributes.cpp b/src/ui/dialog/object-attributes.cpp index f4abd51887..f4e6110ea8 100644 --- a/src/ui/dialog/object-attributes.cpp +++ b/src/ui/dialog/object-attributes.cpp @@ -58,7 +58,6 @@ #include "ui/controller.h" #include "ui/dialog/object-attributes.h" #include "ui/icon-names.h" -#include "ui/menuize.h" #include "ui/pack.h" #include "ui/tools/object-picker-tool.h" #include "ui/syntax.h" @@ -840,7 +839,6 @@ public: action->property_state().signal_changed().connect([=, this]{ int n; action->get_state(n); set_precision(n); }); _main.insert_action_group("attrdialog", std::move(group)); - UI::menuize_popover(*get_widget(builder, "path-menu").get_popover()); get_widget(builder, "path-data-round").signal_clicked().connect([this]{ truncate_digits(_data.get_buffer(), _precision); diff --git a/src/ui/dialog/objects.cpp b/src/ui/dialog/objects.cpp index 9f3abb3f47..cf6c0d5fee 100644 --- a/src/ui/dialog/objects.cpp +++ b/src/ui/dialog/objects.cpp @@ -13,7 +13,6 @@ */ #include -#include #include #include #include @@ -23,12 +22,16 @@ #include #include #include +#include #include +#include #include #include #include #include +#include #include +#include #include #include "objects.h" @@ -39,15 +42,10 @@ #include "document.h" #include "document-undo.h" #include "filter-chemistry.h" -#include "include/gtkmm_version.h" #include "inkscape.h" #include "inkscape-window.h" #include "layer-manager.h" #include "message-stack.h" -#include "object/filters/blend.h" -#include "object/filters/gaussian-blur.h" -#include "object/sp-clippath.h" -#include "object/sp-mask.h" #include "object/sp-root.h" #include "object/sp-shape.h" #include "style-enums.h" @@ -56,12 +54,10 @@ #include "ui/builder-utils.h" #include "ui/contextmenu.h" #include "ui/controller.h" -#include "ui/dialog-events.h" #include "ui/icon-loader.h" #include "ui/icon-names.h" #include "ui/pack.h" #include "ui/popup-menu.h" -#include "ui/selected-color.h" #include "ui/shortcuts.h" #include "ui/tools/node-tool.h" #include "ui/util.h" @@ -121,9 +117,8 @@ private: class ObjectWatcher : public Inkscape::XML::NodeObserver { public: - ObjectWatcher() = delete; ObjectWatcher(ObjectsPanel *panel, SPItem *, Gtk::TreeRow *row, bool is_filtered); - ~ObjectWatcher() final; + ~ObjectWatcher() override; void initRowInfo(); void updateRowInfo(); @@ -799,7 +794,6 @@ ObjectsPanel::ObjectsPanel() return true; }, false); // before - _object_menu.set_parent(*this); _object_menu.signal_closed().connect([this]{ _item_state_toggler->set_force_visible(false); _tree.queue_draw(); @@ -970,12 +964,9 @@ ObjectsPanel::ObjectsPanel() .end = sigc::mem_fun(*this, &ObjectsPanel::on_drag_end ) }, Gtk::PropagationPhase::CAPTURE); - std::vector types; - types.emplace_back(Glib::Value::value_type()); - Controller::add_drop_target(_tree, { .actions = Gdk::DragAction::MOVE, - .types = types, + .types = { Glib::Value::value_type() }, .motion = sigc::mem_fun(*this, &ObjectsPanel::on_drag_motion), .drop = sigc::mem_fun(*this, &ObjectsPanel::on_drag_drop ) }, Gtk::PropagationPhase::CAPTURE); @@ -989,6 +980,7 @@ ObjectsPanel::ObjectsPanel() _scroller.set_child(_tree); _scroller.set_policy( Gtk::PolicyType::AUTOMATIC, Gtk::PolicyType::AUTOMATIC ); _scroller.set_has_frame(true); + _scroller.set_vexpand(); Gtk::Requisition sreq; Gtk::Requisition sreq_natural; _scroller.get_preferred_size(sreq_natural, sreq); @@ -998,12 +990,15 @@ ObjectsPanel::ObjectsPanel() _scroller.set_size_request(sreq.get_width(), minHeight); } - UI::pack_start(_page, header, false, true); - UI::pack_end(_page, _scroller, UI::PackOptions::expand_widget); - UI::pack_start(*this, _page, UI::PackOptions::expand_widget); + _page.append(header); + _page.append(_scroller); + _popoverbin.setChild(&_page); + _popoverbin.set_expand(); + append(_popoverbin); - auto const set_selection_color = [&] - { selection_color = get_color_with_class(_tree, "theme_selected_bg_color"); }; + auto const set_selection_color = [&] { + selection_color = get_color_with_class(_tree, "theme_selected_bg_color"); + }; set_selection_color(); auto enter_layer_label_editing_mode = [this]() { @@ -1035,13 +1030,6 @@ ObjectsPanel::ObjectsPanel() ObjectsPanel::~ObjectsPanel() = default; -void ObjectsPanel::size_allocate_vfunc(int const width, int const height, int const baseline) -{ - parent_type::size_allocate_vfunc(width, height, baseline); - - _object_menu.present(); -} - void ObjectsPanel::desktopReplaced() { layer_changed.disconnect(); @@ -1307,6 +1295,7 @@ bool ObjectsPanel::blendModePopup(int const x, int const y, Gtk::TreeModel::Row _item_state_toggler->set_force_visible(true); + _popoverbin.setPopover(&_object_menu); UI::popup_at(_object_menu, _tree, x, y); return true; } @@ -1685,10 +1674,10 @@ Gtk::EventSequenceState ObjectsPanel::on_click(Gtk::GestureClick const &gesture, } // true == hide menu item for opening this dialog! - auto menu = std::make_shared(getDesktop(), item, true); + auto menu = Gtk::make_managed(getDesktop(), item, true); // popup context menu pointing to the clicked tree row: + _popoverbin.setPopover(menu); UI::popup_at(*menu, _tree, ex, ey); - UI::on_hide_reset(std::move(menu)); } else if (should_set_current_layer()) { getDesktop()->layerManager().setCurrentLayer(item, true); } else { diff --git a/src/ui/dialog/objects.h b/src/ui/dialog/objects.h index 02ac8dfff0..4d9fae7188 100644 --- a/src/ui/dialog/objects.h +++ b/src/ui/dialog/objects.h @@ -29,13 +29,13 @@ #include #include -#include "color-rgba.h" #include "helper/auto-connection.h" #include "preferences.h" #include "selection.h" #include "style-enums.h" #include "ui/dialog/dialog-base.h" #include "ui/widget/color-picker.h" +#include "ui/widget/popover-bin.h" #include "ui/widget/preferences-widget.h" #include "xml/node-observer.h" @@ -84,23 +84,19 @@ enum SelectionStates : SelectionState { /** * A panel that displays objects. */ -class ObjectsPanel final : public DialogBase +class ObjectsPanel : public DialogBase { - using parent_type = DialogBase; - public: ObjectsPanel(); - ~ObjectsPanel() final; + ~ObjectsPanel() override; class ModelColumns; private: - void size_allocate_vfunc(int width, int height, int baseline) final; - - void desktopReplaced() final; - void documentReplaced() final; + void desktopReplaced() override; + void documentReplaced() override; void layerChanged(SPObject *obj); - void selectionChanged(Selection *selected) final; + void selectionChanged(Selection *selected) override; ObjectWatcher *unpackToObject(SPObject *item); // Accessed by ObjectWatcher directly (friend class) @@ -159,6 +155,7 @@ private: Inkscape::auto_connection _tree_style; Inkscape::UI::Widget::ColorPicker _color_picker; Gtk::TreeRow _clicked_item_row; + UI::Widget::PopoverBin _popoverbin; Gtk::Button *_addBarButton(char const* iconName, char const* tooltip, char const *action_name); diff --git a/src/ui/dialog/svg-fonts-dialog.cpp b/src/ui/dialog/svg-fonts-dialog.cpp index d31a9708ea..c68d92ea99 100644 --- a/src/ui/dialog/svg-fonts-dialog.cpp +++ b/src/ui/dialog/svg-fonts-dialog.cpp @@ -263,7 +263,7 @@ Gtk::Box* SvgFontsDialog::AttrCombo(gchar* lbl, const SPAttr /*attr*/){ } GlyphMenuButton::GlyphMenuButton() - : _menu{std::make_unique(*this, Gtk::PositionType::BOTTOM)} + : _menu{std::make_unique(Gtk::PositionType::BOTTOM)} { _label.set_width_chars(2); @@ -284,7 +284,7 @@ void GlyphMenuButton::update(SPFont const * const spfont) { set_sensitive(false); _label.set_label({}); - _menu->delete_all(); + _menu->remove_all(); if (!spfont) return; diff --git a/src/ui/dialog/swatches.cpp b/src/ui/dialog/swatches.cpp index 126b947ca9..7f017c367f 100644 --- a/src/ui/dialog/swatches.cpp +++ b/src/ui/dialog/swatches.cpp @@ -74,8 +74,7 @@ SwatchesPanel::SwatchesPanel(bool compact, char const *prefsPath) _grid_btn(get_widget(_builder, "grid")), _selector(get_widget(_builder, "selector")), _selector_label(get_widget(_builder, "selector-label")), - _selector_menu{compact ? nullptr - : std::make_unique(_selector, Gtk::PositionType::BOTTOM)}, + _selector_menu{compact ? nullptr : std::make_unique(Gtk::PositionType::BOTTOM)}, _new_btn(get_widget(_builder, "new")), _edit_btn(get_widget(_builder, "edit")), _delete_btn(get_widget(_builder, "delete")) @@ -662,7 +661,7 @@ void SwatchesPanel::update_selector_menu() _selector.set_sensitive(false); _selector_label.set_label({}); - _selector_menu->delete_all(); + _selector_menu->remove_all(); if (_palettes.empty()) return; diff --git a/src/ui/dialog/xml-tree.cpp b/src/ui/dialog/xml-tree.cpp index 237bf0fd0f..b074312e9a 100644 --- a/src/ui/dialog/xml-tree.cpp +++ b/src/ui/dialog/xml-tree.cpp @@ -20,8 +20,6 @@ #include "xml-tree.h" -#include -#include #include #include #include @@ -42,17 +40,13 @@ #include "document-undo.h" #include "inkscape.h" #include "layer-manager.h" -#include "message-context.h" -#include "message-stack.h" #include "selection.h" #include "preferences.h" #include "object/sp-root.h" -#include "object/sp-string.h" #include "ui/builder-utils.h" #include "ui/dialog-events.h" #include "ui/icon-loader.h" #include "ui/icon-names.h" -#include "ui/menuize.h" #include "ui/pack.h" #include "ui/syntax.h" #include "ui/tools/tool-base.h" @@ -198,7 +192,6 @@ XmlTree::XmlTree() tooltip->set_text(tip); return true; }, true); - UI::menuize_popover(*popup.get_popover()); auto set_layout = [=, this] (DialogLayout layout) { Glib::ustring icon = "layout-auto"; diff --git a/src/ui/menuize.cpp b/src/ui/menuize.cpp deleted file mode 100644 index fee6f3b6c3..0000000000 --- a/src/ui/menuize.cpp +++ /dev/null @@ -1,183 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/** @file - * Helper functions to make children in GtkPopovers act like GtkMenuItem of GTK3 - */ -/* - * Authors: - * Daniel Boles - * - * Copyright (C) 2023 Daniel Boles - * - * Released under GNU GPL v2+, read the file 'COPYING' for more information. - */ - -#include "menuize.h" - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "ui/util.h" - -namespace Inkscape::UI { - -// Now that our PopoverMenu is scrollable, we want to distinguish between the pointer really moving -// into or within a menu item, versus the pointer staying still but the item being moved beneath it -// …so while I would welcome a nicer way to do that, this is the solution Iʼve come up with for now -// Without GTK supplying absolute coordinates or a ‘synthesised event’ flag I canʼt see another way -[[nodiscard]] static bool pointer_has_moved(Gtk::Widget const &widget) -{ - static std::optional old_x, old_y; - auto &window = dynamic_cast(*widget.get_root()); - auto const surface = window.get_surface(); - auto const device = surface->get_display()->get_default_seat()->get_pointer(); - double new_x{}, new_y{}; Gdk::ModifierType state{}; - surface->get_device_position(device, new_x, new_y, state); - return std::exchange(old_x, new_x) != new_x | std::exchange(old_y, new_y) != new_y; // NOT `||` -} - -static Gtk::Widget &get_widget(GtkEventControllerMotion * const motion) -{ - auto const widget = gtk_event_controller_get_widget(GTK_EVENT_CONTROLLER(motion)); - g_assert(widget != nullptr); - return *Glib::wrap(widget); -} - -static void unset_state(Gtk::Widget &widget) -{ - widget.unset_state_flags(Gtk::StateFlags::FOCUSED | Gtk::StateFlags::PRELIGHT); -} - -static void on_motion_grab_focus(GtkEventControllerMotion * const motion, double /*x*/, double /*y*/, - void * const user_data) -{ - auto &widget = get_widget(motion); - - // If pointer didnʼt move, we got here from a synthesised enter: un-hover item *after* GTK does - // Sadly it also catches item that ends under pointer after scroll, but I donʼt know how to fix - if (bool const is_enter = GPOINTER_TO_INT(user_data); - is_enter && !pointer_has_moved(widget)) - { - Glib::signal_idle().connect_once([&]{ unset_state(widget); }); - return; - } - - if (widget.has_focus()) return; - widget.grab_focus(); // Weʼll then run the below handler @ notify::has-focus -} - -static void on_leave_unset_state(GtkEventControllerMotion * const motion, double /*x*/, double /*y*/, - void * /*user_data*/) -{ - auto &widget = get_widget(motion); - if (!pointer_has_moved(widget)) return; - auto &parent = dynamic_cast(*widget.get_parent()); - unset_state(widget); // This is somehow needed for GtkPopoverMenu, although not our PopoverMenu - unset_state(parent); // Try to unset state on all other menu items, in case we left by keyboard -} - -void menuize(Gtk::Widget &widget) -{ - // If hovered naturally or below, key-focus self & clear focus+hover on rest - auto const motion = gtk_event_controller_motion_new(); - gtk_event_controller_set_propagation_phase(motion, GTK_PHASE_TARGET); - g_signal_connect(motion, "enter" , G_CALLBACK(on_motion_grab_focus), GINT_TO_POINTER(TRUE )); - g_signal_connect(motion, "motion", G_CALLBACK(on_motion_grab_focus), GINT_TO_POINTER(FALSE)); - g_signal_connect(motion, "leave" , G_CALLBACK(on_leave_unset_state), NULL); - gtk_widget_add_controller(widget.gobj(), motion); - - // If key-focused in/out, ‘fake’ correspondingly appearing as hovered or not - widget.property_has_focus().signal_changed().connect([&] - { - if (widget.has_focus()) { - widget.set_state_flags(Gtk::StateFlags::PRELIGHT, false); - } else { - widget.unset_state_flags(Gtk::StateFlags::PRELIGHT); - } - }); -} - -template -static void menuize_all(Gtk::Widget &parent) -{ - for_each_descendant(parent, [](Gtk::Widget &child) - { - if (dynamic_cast(&child)) { - menuize(child); - } - return ForEachResult::_continue; - }); -} - -static void menuize_all(Gtk::Widget &parent, Glib::ustring const &css_name) -{ - for_each_descendant(parent, [&](Gtk::Widget &child) - { - if (auto const klass = GTK_WIDGET_GET_CLASS(child.gobj()); - gtk_widget_class_get_css_name(klass) == css_name) - { - menuize(child); - } - return ForEachResult::_continue; - }); -} - -void autohide_tooltip(Gtk::Popover &popover) -{ - popover.signal_show().connect([&] - { - if (auto const parent = popover.get_parent()) { - parent->set_has_tooltip(false); - } - }); - popover.signal_closed().connect([&] - { - if (auto const parent = popover.get_parent()) { - parent->set_has_tooltip(true); - } - }); -} - -void menuize_popover(Gtk::Popover &popover) -{ - static Glib::ustring const css_class = "menuize"; - - if (popover.has_css_class(css_class)) return; - - popover.add_css_class(css_class); - menuize_all(popover, "modelbutton"); - autohide_tooltip(popover); - - if (auto const popover_menu = dynamic_cast(&popover)) { - popover_menu->set_flags(Gtk::PopoverMenu::Flags::NESTED); - } -} - -std::unique_ptr -make_menuized_popover(Glib::RefPtr model, Gtk::Widget &parent) -{ - auto popover = std::make_unique(model, Gtk::PopoverMenu::Flags::NESTED); - popover->set_parent(parent); - menuize_popover(*popover); - return popover; -} - -} // namespace Inkscape::UI - -/* - Local Variables: - mode:c++ - c-file-style:"stroustrup" - c-file-offsets:((innamespace . 0)(inline-open . 0)(case-label . +)) - indent-tabs-mode:nil - fill-column:99 - End: -*/ -// vim: filetype=cpp:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:fileencoding=utf-8:textwidth=99 : diff --git a/src/ui/menuize.h b/src/ui/menuize.h deleted file mode 100644 index 161b137031..0000000000 --- a/src/ui/menuize.h +++ /dev/null @@ -1,59 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/** @file - * Helper functions to make children in GtkPopovers act like GtkMenuItem of GTK3 - */ -/* - * Authors: - * Daniel Boles - * - * Copyright (C) 2023 Daniel Boles - * - * Released under GNU GPL v2+, read the file 'COPYING' for more information. - */ - -#ifndef SEEN_UI_MENUIZE_H -#define SEEN_UI_MENUIZE_H - -#include -#include - -namespace Gio { -class MenuModel; -} // namespace Gio - -namespace Gtk { -class Popover; -class Widget; -} // namespace Gtk - -namespace Inkscape::UI { - -/// Make items behave like GtkMenu: focus if hovered & style focus+hover same by -/// * If hovered by pointer, grab key focus on self & clear focus+hover on rest; -/// * If key-focused in/out, ‘fake’ correspondingly appearing as hovered or not. -void menuize(Gtk::Widget &widget); - -/// Temporarily disable :relative-to widget tooltip @ ::show; restore @ ::closed -void autohide_tooltip(Gtk::Popover &popover); - -/// menuize() all ModelButtons in @a Popover -void menuize_popover(Gtk::Popover &popover); - -/// Create Popover bound to model, relative to the parent widget, with menuize()d ModelButtons -[[nodiscard]] std::unique_ptr - make_menuized_popover(Glib::RefPtr model, Gtk::Widget &parent); - -} // namespace Inkscape::UI - -#endif // SEEN_UI_MENUIZE_H - -/* - Local Variables: - mode:c++ - c-file-style:"stroustrup" - c-file-offsets:((innamespace . 0)(inline-open . 0)(case-label . +)) - indent-tabs-mode:nil - fill-column:99 - End: -*/ -// vim: filetype=cpp:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:fileencoding=utf-8:textwidth=99 : diff --git a/src/ui/popup-menu.cpp b/src/ui/popup-menu.cpp index 437a37dcde..f43a377176 100644 --- a/src/ui/popup-menu.cpp +++ b/src/ui/popup-menu.cpp @@ -43,34 +43,28 @@ static bool on_key_pressed(unsigned const keyval, unsigned /*keycode*/, Gdk::Mod return false; } -static Gtk::EventSequenceState on_click_pressed(Gtk::GestureClick const &click, - int const n_press, double const x, double const y, - PopupMenuSlot const &slot) +static void on_click_pressed(int n_press, double x, double y, Gtk::GestureClick &click, PopupMenuSlot const &slot) { auto const event = click.get_current_event(); - g_return_val_if_fail(event, Gtk::EventSequenceState::NONE); - if (event->triggers_context_menu()) { - auto const click = PopupMenuClick{n_press, x, y}; - if (slot(click)) return Gtk::EventSequenceState::CLAIMED; + auto const mc = PopupMenuClick{n_press, x, y}; + if (slot(mc)) { + click.set_state(Gtk::EventSequenceState::CLAIMED); + } } - - return Gtk::EventSequenceState::NONE; } void on_popup_menu(Gtk::Widget &widget, PopupMenuSlot slot) { auto key = Gtk::EventControllerKey::create(); key->signal_key_pressed().connect(sigc::bind(&on_key_pressed, slot), true); // after - widget.add_controller(std::move(key)); + widget.add_controller(key); - Controller::add_click(widget, sigc::bind(&on_click_pressed, std::move(slot)), {}, - Controller::Button::any, Gtk::PropagationPhase::TARGET); // ←beat Entry popup handler -} - -sigc::connection on_hide_reset(std::shared_ptr widget) -{ - return widget->signal_hide().connect( [widget = std::move(widget)]() mutable { widget.reset(); }); + auto click = Gtk::GestureClick::create(); + click->set_button(0); + click->set_propagation_phase(Gtk::PropagationPhase::CAPTURE); // before GTK's popup handler + click->signal_pressed().connect(sigc::bind(&on_click_pressed, std::ref(*click), std::move(slot))); + widget.add_controller(click); } static void popup_at(Gtk::Popover &popover, Gtk::Widget &widget, diff --git a/src/ui/popup-menu.h b/src/ui/popup-menu.h index 626bc401c9..6c3c3ca082 100644 --- a/src/ui/popup-menu.h +++ b/src/ui/popup-menu.h @@ -33,12 +33,10 @@ class Popover; class Widget; } // namespace Gtk -namespace Inkscape { - -namespace UI { +namespace Inkscape::UI { /// Information from a GestureClick if a popup menu was opened by click -struct PopupMenuClick final { int const n_press{}; double const x{}, y{}; }; +struct PopupMenuClick { int n_press{}; double x{}, y{}; }; /// Optional: not present if popup wasnʼt triggered by click. using PopupMenuOptionalClick = std::optional; @@ -51,10 +49,6 @@ using PopupMenuSlot = sigc::slot; /// * The right mouse button or other platform convention, as per gtk_event_triggers_context_menu() void on_popup_menu(Gtk::Widget &widget, PopupMenuSlot slot); -/// Connects ::hide of widget to reset() the shared_ptr i.e. to ‘self-destruct’. -/// @returns A connection that can be used to disconnect & prevent self-destruct -sigc::connection on_hide_reset(std::shared_ptr widget); - /// Replace Gtk::Menu::popup_at_pointer. If x or y /// offsets != 0, :pointing-to is set to {x,y,1,1} /// else it is set to the full allocation of @a widget, translated into the coordinate space of the @@ -73,9 +67,7 @@ void popup_at_center(Gtk::Popover &popover, Gtk::Widget &widget); /// @a rect must be valid within the coords of @widget, & @awidget must be a descendant of @parent. void popup_at(Gtk::Popover &popover, Gtk::Widget &widget, Gdk::Rectangle const &rect); -} // namespace UI - -} // namespace Inkscape +} // namespace Inkscape::UI #endif // SEEN_UI_POPUP_MENU_H diff --git a/src/ui/toolbar/page-toolbar.cpp b/src/ui/toolbar/page-toolbar.cpp index 681def39ed..8d275764af 100644 --- a/src/ui/toolbar/page-toolbar.cpp +++ b/src/ui/toolbar/page-toolbar.cpp @@ -121,7 +121,6 @@ PageToolbar::PageToolbar(SPDesktop *desktop) _margin_popover.set_name("MarginPopover"); _margin_popover.set_parent(*this); - signal_destroy().connect([this] { _margin_popover.unparent(); }); // Unparenting in destructor is too late. _text_page_margins.signal_icon_press().connect([&](Gtk::Entry::IconPosition) { if (auto page = _document->getPageManager().getSelected()) { diff --git a/src/ui/toolbar/tool-toolbar.cpp b/src/ui/toolbar/tool-toolbar.cpp index 3ef50219eb..629816b767 100644 --- a/src/ui/toolbar/tool-toolbar.cpp +++ b/src/ui/toolbar/tool-toolbar.cpp @@ -5,7 +5,7 @@ */ /* Authors: * Mike Kowalski (Popovers) - * Matrin Owens (Tool button catagories) + * Martin Owens (Tool button categories) * Jonathon Neuhauser (Open tool preferences) * Tavmjong Bah * @@ -14,7 +14,6 @@ * Released under GNU GPL v2+, read the file 'COPYING' for more information. */ -#include #include #include #include @@ -33,7 +32,6 @@ #include "inkscape-window.h" #include "ui/builder-utils.h" #include "ui/controller.h" -#include "ui/pack.h" #include "ui/util.h" #include "ui/widget/popover-menu.h" #include "ui/widget/popover-menu-item.h" @@ -51,7 +49,9 @@ ToolToolbar::ToolToolbar(InkscapeWindow *window) attachHandlers(builder, window); - UI::pack_start(*this, tool_toolbar, true, true); + _popoverbin.setChild(&tool_toolbar); + _popoverbin.setPopover(_context_menu.get()); + append(_popoverbin); // Hide/show buttons based on preferences. Inkscape::Preferences *prefs = Inkscape::Preferences::get(); @@ -106,7 +106,7 @@ void ToolToolbar::set_visible_buttons(Gtk::ScrolledWindow &tool_toolbar) } // We should avoid passing in the window in Gtk4 by turning "tool_preferences()" into an action. -std::unique_ptr ToolToolbar::makeContextMenu(InkscapeWindow * const window) +std::unique_ptr ToolToolbar::makeContextMenu(InkscapeWindow *window) { Glib::ustring icon_name; Inkscape::Preferences *prefs = Inkscape::Preferences::get(); @@ -114,21 +114,18 @@ std::unique_ptr ToolToolbar::makeContextMenu(InkscapeWi icon_name = "preferences-system"; } - auto &item = *Gtk::make_managed(_("Open tool preferences"), false, - icon_name); - item.signal_activate().connect([=, this] - { + auto &item = *Gtk::make_managed(_("Open tool preferences"), false, icon_name); + item.signal_activate().connect([=, this] { tool_preferences(_context_menu_tool_name, window); _context_menu_tool_name.clear(); }); - auto menu = std::make_unique(*this, Gtk::PositionType::BOTTOM); + auto menu = std::make_unique(Gtk::PositionType::BOTTOM); menu->append(item); return menu; } -void ToolToolbar::showContextMenu(InkscapeWindow * const window, - Gtk::Button &button, Glib::ustring const &tool_name) +void ToolToolbar::showContextMenu(InkscapeWindow *window, Gtk::Button &button, Glib::ustring const &tool_name) { _context_menu_tool_name = tool_name; _context_menu->popup_at_center(button); diff --git a/src/ui/toolbar/tool-toolbar.h b/src/ui/toolbar/tool-toolbar.h index fbeb1307ad..f00831eb23 100644 --- a/src/ui/toolbar/tool-toolbar.h +++ b/src/ui/toolbar/tool-toolbar.h @@ -17,6 +17,7 @@ #include #include "preferences.h" +#include "ui/widget/popover-bin.h" namespace Gtk { class Builder; @@ -54,6 +55,7 @@ private: std::unique_ptr _context_menu; Glib::ustring _context_menu_tool_name; + UI::Widget::PopoverBin _popoverbin; Inkscape::PrefObserver buttons_pref_observer; }; diff --git a/src/ui/toolbar/toolbars.cpp b/src/ui/toolbar/toolbars.cpp index 863376899a..b40cbf19ce 100644 --- a/src/ui/toolbar/toolbars.cpp +++ b/src/ui/toolbar/toolbars.cpp @@ -119,7 +119,6 @@ void Toolbars::create_toolbars(SPDesktop *desktop) auto const sub_toolbox = Gtk::manage(aux_toolboxes[i].create(desktop).release()); sub_toolbox->set_name("SubToolBox"); sub_toolbox->set_hexpand(); - sub_toolbox->set_overflow(Gtk::Overflow::HIDDEN); // Use a grid to wrap the toolbar and a possible swatch. auto const grid = Gtk::make_managed(); diff --git a/src/ui/tools/tool-base.cpp b/src/ui/tools/tool-base.cpp index 00920af984..27efd7c2bc 100644 --- a/src/ui/tools/tool-base.cpp +++ b/src/ui/tools/tool-base.cpp @@ -58,6 +58,7 @@ #include "ui/tools/select-tool.h" #include "ui/widget/canvas.h" #include "ui/widget/canvas-grid.h" +#include "ui/widget/desktop-widget.h" #include "ui/widget/events/canvas-event.h" #include "ui/widget/events/debug.h" @@ -1239,19 +1240,18 @@ void ToolBase::menu_popup(CanvasEvent const &event, SPObject *obj) } } - auto const popup = [&](auto const &event) - { - auto menu = std::make_shared(_desktop, obj); - UI::popup_at(*menu, *_desktop->getCanvas(), event.orig_pos); - UI::on_hide_reset(std::move(menu)); + auto const popup = [&] (std::optional const &pos) { + auto menu = Gtk::make_managed(_desktop, obj); + _desktop->getDesktopWidget()->get_canvas_grid()->setPopover(menu); + UI::popup_at(*menu, *_desktop->getCanvas(), pos); }; inspect_event(event, [&] (ButtonPressEvent const &event) { - popup(event); + popup(event.orig_pos); }, [&] (KeyPressEvent const &event) { - popup(event); + popup(event.orig_pos); }, [&] (CanvasEvent const &event) {} ); diff --git a/src/ui/widget/bin.cpp b/src/ui/widget/bin.cpp index cfcc23cc0b..4bb4330f39 100644 --- a/src/ui/widget/bin.cpp +++ b/src/ui/widget/bin.cpp @@ -15,33 +15,28 @@ namespace Inkscape::UI::Widget { +void Bin::_construct() +{ + set_name("Bin"); + set_overflow(Gtk::Overflow::HIDDEN); + containerize(*this); +} + Bin::Bin(Gtk::Widget *child) { + _construct(); set_child(child); - _connectDestroy(); } Bin::Bin(BaseObjectType *cobject, Glib::RefPtr const &) : Gtk::Widget(cobject) { - _connectDestroy(); - + _construct(); // Add child from builder file. (For custom types, C++ wrapper must already be instantiated.) _child = get_first_child(); assert(!_child || !_child->get_next_accessible_sibling()); } -void Bin::_connectDestroy() -{ - // This signal may fire just before destruction to tell us to unparent all children. - signal_destroy().connect([this] { unset_child(); }); -} - -Bin::~Bin() -{ - unset_child(); -} - void Bin::set_child(Gtk::Widget *child) { if (child == _child || (child && child->get_parent())) { @@ -64,7 +59,10 @@ void Bin::on_size_allocate(int width, int height, int baseline) Gtk::Widget::size_allocate_vfunc(width, height, baseline); if (_child && _child->get_visible()) { - _child->size_allocate({0, 0, width, height}, baseline); + int min_width, min_height, ignore; + _child->measure(Gtk::Orientation::HORIZONTAL, -1, min_width, ignore, ignore, ignore); + _child->measure(Gtk::Orientation::VERTICAL, -1, min_height, ignore, ignore, ignore); + _child->size_allocate({0, 0, std::max(width, min_width), std::max(height, min_height)}, baseline); } } @@ -73,19 +71,16 @@ Gtk::SizeRequestMode Bin::get_request_mode_vfunc() const return _child ? _child->get_request_mode() : Gtk::SizeRequestMode::CONSTANT_SIZE; } -void Bin::measure_vfunc(Gtk::Orientation orientation, int const for_size, - int &minimum, int &natural, - int &minimum_baseline, int &natural_baseline) const +void Bin::measure_vfunc(Gtk::Orientation orientation, int for_size, int &min, int &nat, int &min_baseline, int &nat_baseline) const { if (_child && _child->get_visible()) { - _child->measure(orientation, for_size, - minimum, natural, minimum_baseline, natural_baseline); + _child->measure(orientation, for_size, min, nat, min_baseline, nat_baseline); } else { - minimum = natural = minimum_baseline = natural_baseline = 0; + min = nat = min_baseline = nat_baseline = 0; } } -void Bin::size_allocate_vfunc(int const width, int const height, int const baseline) +void Bin::size_allocate_vfunc(int width, int height, int baseline) { _signal_before_resize.emit(width, height, baseline); on_size_allocate(width, height, baseline); diff --git a/src/ui/widget/bin.h b/src/ui/widget/bin.h index 8fe4462211..042513ff08 100644 --- a/src/ui/widget/bin.h +++ b/src/ui/widget/bin.h @@ -14,6 +14,7 @@ #define INKSCAPE_UI_WIDGET_BIN_H #include +#include "ui/containerize.h" namespace Gtk { class Builder; } @@ -31,7 +32,6 @@ class Bin : public Gtk::Widget public: Bin(Gtk::Widget *child = nullptr); Bin(BaseObjectType *cobject, Glib::RefPtr const &); - ~Bin() override; /// Gets the child widget, or nullptr if none. Gtk::Widget *get_child() { return _child; } @@ -42,10 +42,10 @@ public: /// Sets (parents) the child widget, or unsets (unparents) it if @a child is null. void set_child(Gtk::Widget *child); - /// Sets (parents) the child widget. + /// Convenience function: Sets (parents) the child widget. void set_child(Gtk::Widget &child) { set_child(&child); } - /// Unsets (unparents) the child widget. + /// Convenience function: Unsets (unparents) the child widget. void unset_child() { set_child(nullptr); } /// Register a handler to run immediately before a resize operation. @@ -65,13 +65,13 @@ protected: virtual void on_size_allocate(int width, int height, int baseline); private: + void _construct(); + Gtk::Widget *_child = nullptr; sigc::signal _signal_before_resize; sigc::signal _signal_after_resize; - void _connectDestroy(); - Gtk::SizeRequestMode get_request_mode_vfunc() const override; void size_allocate_vfunc(int width, int height, int baseline) override; diff --git a/src/ui/widget/canvas-grid.cpp b/src/ui/widget/canvas-grid.cpp index 25a1b7c488..4880b5f493 100644 --- a/src/ui/widget/canvas-grid.cpp +++ b/src/ui/widget/canvas-grid.cpp @@ -75,22 +75,22 @@ CanvasGrid::CanvasGrid(SPDesktopWidget *dtw) _notice = CanvasNotice::create(); // Canvas overlay - _canvas_overlay.set_child(*_canvas); + _popoverbin.setChild(_canvas.get()); + _canvas_overlay.set_child(_popoverbin); _canvas_overlay.add_overlay(_command_palette->get_base_widget()); _canvas_overlay.add_overlay(*_notice); + _canvas_overlay.set_expand(); // Horizontal Ruler _hruler = std::make_unique(Gtk::Orientation::HORIZONTAL); _hruler->add_track_widget(*_canvas); _hruler->set_hexpand(true); - _hruler->set_visible(true); // Tooltip/Unit set elsewhere // Vertical Ruler _vruler = std::make_unique(Gtk::Orientation::VERTICAL); _vruler->add_track_widget(*_canvas); _vruler->set_vexpand(true); - _vruler->set_visible(true); // Tooltip/Unit set elsewhere. // Guide Lock @@ -111,6 +111,7 @@ CanvasGrid::CanvasGrid(SPDesktopWidget *dtw) _subgrid.attach(*_vruler, 0, 1, 1, 1); _subgrid.attach(*_hruler, 1, 0, 1, 1); _subgrid.attach(_canvas_overlay, 1, 1, 1, 1); + _subgrid.set_expand(); // Horizontal Scrollbar _hadj = Gtk::Adjustment::create(0.0, -4000.0, 4000.0, 10.0, 100.0, 4.0); diff --git a/src/ui/widget/canvas-grid.h b/src/ui/widget/canvas-grid.h index a2f4cf7396..a5dc546bc1 100644 --- a/src/ui/widget/canvas-grid.h +++ b/src/ui/widget/canvas-grid.h @@ -26,6 +26,7 @@ #include "display/control/canvas-item-ptr.h" #include "helper/auto-connection.h" +#include "ui/widget/popover-bin.h" namespace Gtk { class Adjustment; @@ -58,13 +59,13 @@ class Ruler; * A Gtk::Grid widget that contains rulers, scrollbars, buttons, and, of course, the canvas. * Canvas has an overlay to let us put stuff on the canvas. */ -class CanvasGrid final : public Gtk::Grid +class CanvasGrid : public Gtk::Grid { using parent_type = Gtk::Grid; public: CanvasGrid(SPDesktopWidget *dtw); - ~CanvasGrid() final; + ~CanvasGrid() override; void ShowScrollbars(bool state = true); void ToggleScrollbars(); @@ -78,6 +79,8 @@ public: void showNotice(Glib::ustring const &msg, unsigned timeout = 0); + void setPopover(Gtk::Popover *popover) { _popoverbin.setPopover(popover); } + Inkscape::UI::Widget::Canvas *GetCanvas() { return _canvas.get(); }; // Hopefully temp. @@ -98,10 +101,11 @@ public: private: // Signal callbacks - void size_allocate_vfunc(int width, int height, int baseline) final; - void on_realize() final; + void size_allocate_vfunc(int width, int height, int baseline) override; + void on_realize() override; // The widgets + Inkscape::UI::Widget::PopoverBin _popoverbin; std::unique_ptr _canvas; std::unique_ptr _command_palette; CanvasNotice *_notice; diff --git a/src/ui/widget/color-palette.cpp b/src/ui/widget/color-palette.cpp index 58fc3909ba..aabb8a91b1 100644 --- a/src/ui/widget/color-palette.cpp +++ b/src/ui/widget/color-palette.cpp @@ -47,7 +47,7 @@ namespace Inkscape::UI::Widget { auto const config = Gtk::make_managed(_("Configure..."), true); - auto menu = std::make_unique(nullptr, Gtk::PositionType::TOP); + auto menu = std::make_unique(Gtk::PositionType::TOP); menu->add_css_class("ColorPalette"); menu->append(*separator); menu->append(*config); @@ -78,7 +78,6 @@ ColorPalette::ColorPalette(): btn_menu.set_popover(*_menu); auto& dlg = get_settings_popover(); config.signal_activate().connect([&, this] { - dlg.set_parent(btn_menu); dlg.popup(); }); diff --git a/src/ui/widget/color-palette.h b/src/ui/widget/color-palette.h index 9ed10eb727..33f726f4f2 100644 --- a/src/ui/widget/color-palette.h +++ b/src/ui/widget/color-palette.h @@ -20,7 +20,6 @@ #include #include <2geom/int-point.h> -#include "helper/auto-connection.h" #include "ui/widget/palette_t.h" #include "ui/widget/bin.h" @@ -105,7 +104,6 @@ private: void _enable_scrollbar(bool show); void _enable_stretch(bool enable); void _set_large_pinned_panel(bool large); - static gboolean check_scrollbar(gpointer self); void update_checkbox(); void update_stretch(); int get_tile_size(bool horz) const; @@ -148,7 +146,6 @@ private: bool _show_labels = false; int _page_size = 0; Geom::IntPoint _allocation; - auto_connection _idle_resize; }; } // namespace Widget diff --git a/src/ui/widget/completion-popup.cpp b/src/ui/widget/completion-popup.cpp index 5ad62bbd98..0e482618ff 100644 --- a/src/ui/widget/completion-popup.cpp +++ b/src/ui/widget/completion-popup.cpp @@ -25,7 +25,7 @@ CompletionPopup::CompletionPopup() : _builder(create_builder("completion-box.glade")), _search(get_widget(_builder, "search")), _button(get_widget(_builder, "menu-btn")), - _popover_menu{nullptr, Gtk::PositionType::BOTTOM}, + _popover_menu{Gtk::PositionType::BOTTOM}, _completion(get_object(_builder, "completion")) { Controller::add_key<&CompletionPopup::onPopoverKeyPressed>(_popover_menu, *this, Gtk::PropagationPhase::CAPTURE); diff --git a/src/ui/widget/desktop-widget.cpp b/src/ui/widget/desktop-widget.cpp index 2d9cc75c57..a3c946d32e 100644 --- a/src/ui/widget/desktop-widget.cpp +++ b/src/ui/widget/desktop-widget.cpp @@ -38,7 +38,6 @@ #include "conn-avoid-ref.h" #include "desktop.h" -#include "display/control/canvas-item-drawing.h" #include "document.h" #include "document-undo.h" #include "enums.h" @@ -46,15 +45,11 @@ #include "inkscape-window.h" #include "object/sp-image.h" #include "object/sp-namedview.h" -#include "selection.h" -#include "ui/builder-utils.h" #include "ui/dialog/dialog-container.h" #include "ui/dialog/dialog-multipaned.h" #include "ui/dialog/dialog-window.h" #include "ui/dialog-run.h" #include "ui/dialog/swatches.h" -#include "ui/icon-loader.h" -#include "ui/icon-names.h" #include "ui/monitor.h" // Monitor aspect ratio #include "ui/pack.h" #include "ui/shortcuts.h" @@ -67,7 +62,6 @@ #include "ui/util.h" #include "ui/widget/canvas-grid.h" #include "ui/widget/canvas.h" -#include "ui/widget/color-palette.h" #include "ui/widget/combo-tool-item.h" #include "ui/widget/ink-ruler.h" #include "ui/widget/spinbutton.h" diff --git a/src/ui/widget/desktop-widget.h b/src/ui/widget/desktop-widget.h index f2a1e76a09..111620c8c9 100644 --- a/src/ui/widget/desktop-widget.h +++ b/src/ui/widget/desktop-widget.h @@ -90,13 +90,13 @@ class StatusBar; } // namespace Inkscape::UI /// A GtkBox on an SPDesktop. -class SPDesktopWidget final : public Gtk::Box +class SPDesktopWidget : public Gtk::Box { using parent_type = Gtk::Box; public: SPDesktopWidget(InkscapeWindow *inkscape_window, SPDocument *document); - ~SPDesktopWidget() final; + ~SPDesktopWidget() override; Inkscape::UI::Widget::CanvasGrid *get_canvas_grid() { return _canvas_grid; } // Temp, I hope! Inkscape::UI::Widget::Canvas *get_canvas() { return _canvas; } @@ -109,8 +109,8 @@ public: Gio::ActionMap *get_action_map(); - void on_realize() final; - void on_unrealize() final; + void on_realize() override; + void on_unrealize() override; private: Inkscape::auto_connection modified_connection; @@ -118,16 +118,13 @@ private: std::unique_ptr _desktop; InkscapeWindow *_window = nullptr; - // The root vbox of the window layout. - Gtk::Box *_vbox; - Gtk::Paned *_tbbox = nullptr; Gtk::Box *_hbox = nullptr; std::unique_ptr _container; Inkscape::UI::Dialog::DialogMultipaned *_columns = nullptr; Gtk::Grid* _top_toolbars = nullptr; - Inkscape::UI::Widget::StatusBar *_statusbar = nullptr; + Inkscape::UI::Widget::StatusBar *_statusbar = nullptr; Inkscape::UI::Dialog::SwatchesPanel *_panels; /** A grid to display the canvas, rulers, and scrollbars. */ diff --git a/src/ui/widget/gradient-editor.cpp b/src/ui/widget/gradient-editor.cpp index 53a25cb98e..405bfad34a 100644 --- a/src/ui/widget/gradient-editor.cpp +++ b/src/ui/widget/gradient-editor.cpp @@ -159,7 +159,7 @@ Glib::ustring get_repeat_icon(SPGradientSpread mode) { GradientEditor::GradientEditor(const char* prefs) : _builder(Inkscape::UI::create_builder("gradient-edit.glade")), _selector(Gtk::make_managed()), - _repeat_popover{std::make_unique(nullptr, Gtk::PositionType::BOTTOM)}, + _repeat_popover{std::make_unique(Gtk::PositionType::BOTTOM)}, _repeat_icon(get_widget(_builder, "repeatIco")), _stop_tree(get_widget(_builder, "stopList")), _offset_btn(get_widget(_builder, "offsetSpin")), diff --git a/src/ui/widget/ink-ruler.cpp b/src/ui/widget/ink-ruler.cpp index 3d555aae87..d8f5e2e506 100644 --- a/src/ui/widget/ink-ruler.cpp +++ b/src/ui/widget/ink-ruler.cpp @@ -19,11 +19,13 @@ #include #include #include +#include #include #include #include "ink-ruler.h" #include "inkscape.h" +#include "ui/containerize.h" #include "ui/controller.h" #include "ui/popup-menu.h" #include "ui/themes.h" @@ -32,8 +34,8 @@ struct SPRulerMetric { - gdouble ruler_scale[16]; - gint subdivide[5]; + double ruler_scale[16]; + int subdivide[5]; }; // Ruler metric for general use. @@ -70,27 +72,22 @@ Ruler::Ruler(Gtk::Orientation orientation) { set_name("InkRuler"); add_css_class(_orientation == Gtk::Orientation::HORIZONTAL ? "horz" : "vert"); + containerize(*this); + set_layout_manager(Gtk::BinLayout::create()); set_draw_func(sigc::mem_fun(*this, &Ruler::draw_func)); Controller::add_motion(*this, *this); Controller::add_click(*this, sigc::mem_fun(*this, &Ruler::on_click_pressed), {}, Controller::Button::right); - set_visible(false); - auto prefs = Inkscape::Preferences::get(); _watch_prefs = prefs->createObserver("/options/ruler/show_bbox", sigc::mem_fun(*this, &Ruler::on_prefs_changed)); on_prefs_changed(); INKSCAPE.themecontext->getChangeThemeSignal().connect([this]{ css_changed(nullptr); }); - - signal_destroy().connect([this]{ unparent_children(); }); } -Ruler::~Ruler() -{ - unparent_children(); -} +Ruler::~Ruler() = default; void Ruler::on_prefs_changed() { @@ -168,12 +165,6 @@ Ruler::add_track_widget(Gtk::Widget& widget) Gtk::PropagationPhase::TARGET, Controller::When::before); // We connected 1st to event, so continue } -void -Ruler::size_allocate_vfunc(int const width, int const height, int const baseline) -{ - _popover->present(); -} - // Draws marker in response to motion events from canvas. Position is defined in ruler pixel // coordinates. The routine assumes that the ruler is the same width (height) as the canvas. If // not, one could use Gtk::Widget::translate_coordinates() to convert the coordinates. @@ -198,9 +189,7 @@ Ruler::on_motion(GtkEventControllerMotion const * motion, double const x, double queue_draw(); } -Gtk::EventSequenceState -Ruler::on_click_pressed(Gtk::GestureClick const & /*click*/, - int /*n_press*/, double const x, double const y) +Gtk::EventSequenceState Ruler::on_click_pressed(Gtk::GestureClick const &, int, double x, double y) { UI::popup_at(*_popover, *this, x, y); return Gtk::EventSequenceState::CLAIMED; @@ -557,11 +546,6 @@ std::unique_ptr Ruler::create_context_menu() return popover; } -void Ruler::unparent_children() -{ - _popover->unparent(); -} - } // namespace Inkscape::UI::Widget /* diff --git a/src/ui/widget/ink-ruler.h b/src/ui/widget/ink-ruler.h index d5c56874a4..45a2fcff0e 100644 --- a/src/ui/widget/ink-ruler.h +++ b/src/ui/widget/ink-ruler.h @@ -59,22 +59,19 @@ public: void add_track_widget(Gtk::Widget& widget); private: - void size_allocate_vfunc(int width, int height, int baseline) final; - void unparent_children(); - std::pair get_drawing_size(); bool draw_scale(const Cairo::RefPtr<::Cairo::Context>& cr); void draw_marker(const Cairo::RefPtr<::Cairo::Context>& cr); Cairo::RectangleInt marker_rect(); void draw_func(Cairo::RefPtr const &cr, int width, int height); - void css_changed(GtkCssStyleChange *) final; + void css_changed(GtkCssStyleChange *) override; void on_prefs_changed(); void on_motion(GtkEventControllerMotion const *motion, double x, double y); Gtk::EventSequenceState on_click_pressed(Gtk::GestureClick const &click, int n_press, double x, double y); - [[nodiscard]] std::unique_ptr create_context_menu(); + std::unique_ptr create_context_menu(); Cairo::RefPtr draw_label(Cairo::RefPtr const &surface_in, int label_value); Inkscape::PrefObserver _watch_prefs; diff --git a/src/ui/widget/page-properties.cpp b/src/ui/widget/page-properties.cpp index 51abd9696b..f1950627d3 100644 --- a/src/ui/widget/page-properties.cpp +++ b/src/ui/widget/page-properties.cpp @@ -41,7 +41,6 @@ #include "page-size-preview.h" #include "ui/builder-utils.h" -#include "ui/menuize.h" #include "ui/operation-blocker.h" #include "ui/util.h" #include "ui/widget/color-picker.h" @@ -120,7 +119,7 @@ class PagePropertiesBox final : public PageProperties { } menu->append(_("Custom"), get_detailed_action(_page_sizes.size())); // sentinel _templates_popover.set_menu_model(std::move(menu)); - UI::menuize_popover(_templates_popover); + _templates_popover.set_flags(Gtk::PopoverMenu::Flags::NESTED); } public: diff --git a/src/ui/widget/popover-bin.cpp b/src/ui/widget/popover-bin.cpp new file mode 100644 index 0000000000..7c72ddbc93 --- /dev/null +++ b/src/ui/widget/popover-bin.cpp @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include "popover-bin.h" + +#include +#include "ui/containerize.h" + +namespace Inkscape::UI::Widget { + +PopoverBin::PopoverBin() +{ + set_name("PopoverBin"); + set_layout_manager(Gtk::BinLayout::create()); + containerize(*this); +} + +void PopoverBin::_replace(Gtk::Widget *&holder, Gtk::Widget *widget) +{ + if (widget == holder) { + return; + } + + if (holder) { + holder->unparent(); + } + + holder = widget; + + if (holder) { + holder->set_parent(*this); + } +} + +} // namespace Inkscape::UI::Widget diff --git a/src/ui/widget/popover-bin.h b/src/ui/widget/popover-bin.h new file mode 100644 index 0000000000..0c05ffa89f --- /dev/null +++ b/src/ui/widget/popover-bin.h @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#ifndef INKSCAPE_UI_WIDGET_POPOVER_BIN_H +#define INKSCAPE_UI_WIDGET_POPOVER_BIN_H + +#include + +namespace Inkscape::UI::Widget { + +/** + * Holds a single child widget while allowing a single popover to be displayed over it. + * + * Setting another popover displaces (unparents) the old one. If it is managed, this + * typically also causes it to be deleted. + * + * Thus, a useful pattern to deal with popovers that are constructed on-the-fly is to + * create it using Gtk::make_managed(), then attach it using setPopover(). + */ +class PopoverBin : public Gtk::Widget +{ +public: + PopoverBin(); + + void setChild(Gtk::Widget *child) { _replace(_child, child); } + void setPopover(Gtk::Popover *popover) { _replace(_popover, popover); } + +private: + void _replace(Gtk::Widget *&holder, Gtk::Widget *widget); + + Gtk::Widget *_child = nullptr; + Gtk::Widget *_popover = nullptr; +}; + +} // namespace Inkscape::UI::Widget + +#endif // INKSCAPE_UI_WIDGET_POPOVER_BIN_H diff --git a/src/ui/widget/popover-menu-item.cpp b/src/ui/widget/popover-menu-item.cpp index b89ae4c5cb..cac6d06bf9 100644 --- a/src/ui/widget/popover-menu-item.cpp +++ b/src/ui/widget/popover-menu-item.cpp @@ -20,7 +20,6 @@ #include #include -#include "ui/menuize.h" #include "ui/util.h" #include "ui/widget/popover-menu.h" @@ -69,8 +68,6 @@ PopoverMenuItem::PopoverMenuItem(Glib::ustring const &text, } }); } - - UI::menuize(*this); } Glib::SignalProxy PopoverMenuItem::signal_activate() diff --git a/src/ui/widget/popover-menu.cpp b/src/ui/widget/popover-menu.cpp index 9e5387834b..c1becced6a 100644 --- a/src/ui/widget/popover-menu.cpp +++ b/src/ui/widget/popover-menu.cpp @@ -20,7 +20,6 @@ #include #include -#include "ui/menuize.h" #include "ui/popup-menu.h" #include "ui/util.h" #include "ui/widget/css-name-class-init.h" @@ -45,11 +44,7 @@ public: } }; -PopoverMenu::PopoverMenu(Gtk::Widget &parent, Gtk::PositionType const position) -: PopoverMenu{&parent, position} -{} - -PopoverMenu::PopoverMenu(Gtk::Widget *const parent, Gtk::PositionType const position) +PopoverMenu::PopoverMenu(Gtk::PositionType const position) : Glib::ObjectBase{"PopoverMenu"} , Gtk::Popover{} , _scrolled_window{*Gtk::make_managed()} @@ -58,7 +53,6 @@ PopoverMenu::PopoverMenu(Gtk::Widget *const parent, Gtk::PositionType const posi add_css_class("popover-menu"); add_css_class("menu"); - if (parent) set_parent(*parent); set_position(position); _scrolled_window.set_propagate_natural_width (true); @@ -78,9 +72,6 @@ PopoverMenu::PopoverMenu(Gtk::Widget *const parent, Gtk::PositionType const posi // This is also nicer for menus with only 1 item, like the ToolToolbar popup Glib::signal_idle().connect_once( [this]{ unset_items_focus_hover(nullptr); }); }); - - // Temporarily hide tooltip of relative-to widget to avoid it covering us up - UI::autohide_tooltip(*this); } void PopoverMenu::attach(Gtk::Widget &item, @@ -121,16 +112,6 @@ void PopoverMenu::remove(Gtk::Widget &item) _items.erase(it); } -void PopoverMenu::remove_all() -{ - remove_all(false); -} - -void PopoverMenu::delete_all() -{ - remove_all(true); -} - void PopoverMenu::append_section_label(Glib::ustring const &markup) { auto const label = Gtk::make_managed(); @@ -237,14 +218,10 @@ void PopoverMenu::unset_items_focus_hover(Gtk::Widget * const except_active) } } -void PopoverMenu::remove_all(bool const and_delete) +void PopoverMenu::remove_all() { - for (auto const item: _items) { + for (auto item : _items) { _grid.remove(*item); - if (and_delete) { - g_assert(item->is_managed_()); // "Private API", but sanity check for us - delete item; // This is not ideal, but gtkmm/object.cc says should be OK - } } _items.clear(); } diff --git a/src/ui/widget/popover-menu.h b/src/ui/widget/popover-menu.h index 77b11e0725..bb63918655 100644 --- a/src/ui/widget/popover-menu.h +++ b/src/ui/widget/popover-menu.h @@ -40,10 +40,8 @@ class PopoverMenuItem; /// for Menus, including grid and activation functionality. class PopoverMenu final : public Gtk::Popover { public: - /// Create popover with CSS classes `.menu` & `.popover-menu`, - /// positioned as requested vs. relative-to/popup_at() widget. - [[nodiscard]] PopoverMenu(Gtk::Widget *parent, Gtk::PositionType const position); - [[nodiscard]] PopoverMenu(Gtk::Widget &parent, Gtk::PositionType const position); + /// Create popover with CSS classes `.menu` & `.popover-menu`, positioned as requested. + PopoverMenu(Gtk::PositionType position); /// Add child at pos as per Gtk::Menu::attach() void attach(Gtk::Widget &child, @@ -53,12 +51,10 @@ public: void append(Gtk::Widget &child); /// Add new row containing child, at end/bottom void prepend(Gtk::Widget &child); - /// Remove/unparent added child. + /// Remove added child. void remove(Gtk::Widget &child); - /// Remove/unparent all items. If they were Gtk::manage()d, theyʼll regain floating references. + /// Remove all items. void remove_all(); - /// Remove/unparent all items, also calling `delete` on each assuming they were Gtk::manage()d. - void delete_all(); /// Append label, w/ markup & the .dim-label style class. void append_section_label(Glib::ustring const &markup); @@ -97,8 +93,6 @@ private: friend class PopoverMenuItem; friend class CompletionPopup; void unset_items_focus_hover(Gtk::Widget *except_active); - - void remove_all(bool and_delete); }; } // namespace Inkscape::UI::Widget diff --git a/src/ui/widget/selected-style.cpp b/src/ui/widget/selected-style.cpp index 44ce450250..f43eae32a4 100644 --- a/src/ui/widget/selected-style.cpp +++ b/src/ui/widget/selected-style.cpp @@ -55,7 +55,6 @@ #include "util/units.cpp" #include "util-string/ustring-format.h" #include "widgets/paintdef.h" -#include "widgets/spw-utilities.h" static constexpr int SELECTED_STYLE_SB_WIDTH = 48; static constexpr int SELECTED_STYLE_PLACE_WIDTH = 50; @@ -127,9 +126,7 @@ enum ui_drop_target_info { /* convenience function */ static Dialog::FillAndStroke *get_fill_and_stroke_panel(SPDesktop *desktop); -SelectedStyle::SelectedStyle(bool /*layout*/) - : Gtk::Box(Gtk::Orientation::HORIZONTAL) - , dragging(false) +SelectedStyle::SelectedStyle() { set_name("SelectedStyle"); set_size_request (SELECTED_STYLE_WIDTH, -1); @@ -240,10 +237,10 @@ SelectedStyle::SelectedStyle(bool /*layout*/) opacity_sb->signal_value_changed().connect(sigc::mem_fun(*this, &SelectedStyle::on_opacity_changed)); grid->attach(*opacity_label, 4, 0, 1, 2); - grid->attach(*opacity_sb, 5, 0, 1, 2); + grid->attach(*opacity_sb, 5, 0, 1, 2); grid->set_column_spacing(4); - append(*grid); + setChild(grid); make_popup_units(); } @@ -545,6 +542,7 @@ Gtk::EventSequenceState SelectedStyle::on_fill_click(Gtk::GestureClick const &cl if (Dialog::FillAndStroke *fs = get_fill_and_stroke_panel(_desktop)) fs->showPageFill(); } else if (button == 3) { // right-click, popup menu + setPopover(_popup[SS_FILL].get()); _popup[SS_FILL]->popup_at_center(*swatch[SS_FILL]); } else if (button == 2) { // middle click, toggle none/lastcolor if (_mode[SS_FILL] == SS_NONE) { @@ -564,6 +562,7 @@ Gtk::EventSequenceState SelectedStyle::on_stroke_click(Gtk::GestureClick const & if (Dialog::FillAndStroke *fs = get_fill_and_stroke_panel(_desktop)) fs->showPageStrokePaint(); } else if (button == 3) { // right-click, popup menu + setPopover(_popup[SS_STROKE].get()); _popup[SS_STROKE]->popup_at_center(*swatch[SS_STROKE]); } else if (button == 2) { // middle click, toggle none/lastcolor if (_mode[SS_STROKE] == SS_NONE) { @@ -575,18 +574,19 @@ Gtk::EventSequenceState SelectedStyle::on_stroke_click(Gtk::GestureClick const & return Gtk::EventSequenceState::CLAIMED; } -Gtk::EventSequenceState SelectedStyle::on_sw_click(Gtk::GestureClick const &click, int n_press, double /*x*/, - double /*y*/) +Gtk::EventSequenceState SelectedStyle::on_sw_click(Gtk::GestureClick const &click, int n_press, double, double) { auto const button = click.get_current_button(); if (button == 1 && !dragging) { // click, open fill&stroke if (Dialog::FillAndStroke *fs = get_fill_and_stroke_panel(_desktop)) fs->showPageStrokeStyle(); } else if (button == 3) { // right-click, popup menu - auto const it = std::find_if(_unit_mis.cbegin(), _unit_mis.cend(), - [=](auto const mi){ return mi->get_label() == _sw_unit->abbr; }); + auto const it = std::find_if(_unit_mis.cbegin(), _unit_mis.cend(), [this] (auto mi) { + return mi->get_label() == _sw_unit->abbr; + }); if (it != _unit_mis.cend()) (*it)->set_active(true); + setPopover(_popup_sw.get()); _popup_sw->popup_at_center(*stroke_width); } else if (button == 2) { // middle click, toggle none/lastwidth? // @@ -619,8 +619,7 @@ static UI::Widget::PopoverMenuItem *make_menu_item(Glib::ustring const &label, S void SelectedStyle::make_popup(FillOrStroke const i) { - _popup[i] = std::make_unique(*this, Gtk::PositionType::TOP); - + _popup[i] = std::make_unique(Gtk::PositionType::TOP); auto const add_item = [&](Glib::ustring const & fill_label, auto const fill_method, Glib::ustring const &stroke_label, auto const stroke_method) @@ -680,7 +679,7 @@ void SelectedStyle::make_popup(FillOrStroke const i) void SelectedStyle::make_popup_units() { - _popup_sw = std::make_unique(*this, Gtk::PositionType::TOP); + _popup_sw = std::make_unique(Gtk::PositionType::TOP); _popup_sw->append_section_label(_("Stroke Width")); @@ -976,9 +975,10 @@ void SelectedStyle::opacity_1() {opacity_sb->set_value(100);} void SelectedStyle::make_popup_opacity() { - _popup_opacity = std::make_unique(*this, Gtk::PositionType::TOP); - auto const add_item = [&](Glib::ustring const &label, auto const method) - { _popup_opacity->append(*make_menu_item(label, sigc::mem_fun(*this, method))); }; + _popup_opacity = std::make_unique(Gtk::PositionType::TOP); + auto const add_item = [&] (Glib::ustring const &label, auto method) { + _popup_opacity->append(*make_menu_item(label, sigc::mem_fun(*this, method))); + }; add_item(_("0 (Transparent)"), &SelectedStyle::opacity_0 ); add_item(_("25%" ), &SelectedStyle::opacity_025); add_item(_("50%" ), &SelectedStyle::opacity_05 ); @@ -988,6 +988,7 @@ void SelectedStyle::make_popup_opacity() bool SelectedStyle::on_opacity_popup(PopupMenuOptionalClick) { + setPopover(_popup_opacity.get()); _popup_opacity->popup_at_center(*opacity_sb); return true; } diff --git a/src/ui/widget/selected-style.h b/src/ui/widget/selected-style.h index 2402955483..2b08cf5197 100644 --- a/src/ui/widget/selected-style.h +++ b/src/ui/widget/selected-style.h @@ -25,6 +25,7 @@ #include "rotateable.h" #include "ui/popup-menu.h" #include "ui/widget/spinbutton.h" +#include "ui/widget/popover-bin.h" namespace Gtk { class Adjustment; @@ -114,11 +115,11 @@ private: /** * Selected style indicator (fill, stroke, opacity). */ -class SelectedStyle : public Gtk::Box +class SelectedStyle : public UI::Widget::PopoverBin { public: - bool dragging; - SelectedStyle(bool layout = true); + bool dragging = false; + SelectedStyle(); void setDesktop(SPDesktop *desktop); SPDesktop *getDesktop() {return _desktop;} diff --git a/src/ui/widget/spinbutton.cpp b/src/ui/widget/spinbutton.cpp index 999105c3c0..1e030c1ed7 100644 --- a/src/ui/widget/spinbutton.cpp +++ b/src/ui/widget/spinbutton.cpp @@ -20,7 +20,6 @@ #include "scroll-utils.h" #include "ui/controller.h" -#include "ui/menuize.h" #include "ui/tools/tool-base.h" #include "ui/widget/popover-menu-item.h" #include "ui/widget/popover-menu.h" @@ -54,11 +53,12 @@ void SpinButton::_construct() { Controller::add_key<&SpinButton::on_key_pressed>(*this, *this); - property_has_focus().signal_changed().connect( - sigc::mem_fun(*this, &SpinButton::on_has_focus_changed)); + property_has_focus().signal_changed().connect(sigc::mem_fun(*this, &SpinButton::on_has_focus_changed)); UI::on_popup_menu(*this, sigc::mem_fun(*this, &SpinButton::on_popup_menu)); signal_input().connect(sigc::mem_fun(*this, &SpinButton::on_input), true); + + signal_destroy().connect([this] { _unparentChildren(); }); } int SpinButton::on_input(double &newvalue) @@ -176,12 +176,12 @@ bool SpinButton::on_popup_menu(PopupMenuOptionalClick) if (!_custom_popup) { return false; } - auto popover_menu = get_popover_menu(); - popover_menu->popup_at_center(*this); + create_popover_menu(); + _popover_menu->popup_at_center(*this); return true; } -std::shared_ptr SpinButton::get_popover_menu() +void SpinButton::create_popover_menu() { auto adj = get_adjustment(); auto adj_value = adj->get_value(); @@ -201,8 +201,12 @@ std::shared_ptr SpinButton::get_popover_menu() values.emplace(std::fmin(adj_value + page, upper), ""); values.emplace(std::fmax(adj_value - page, lower), ""); - static auto popover_menu = std::make_shared(*this, Gtk::PositionType::BOTTOM); - popover_menu->delete_all(); + if (!_popover_menu) { + _popover_menu = std::make_unique(Gtk::PositionType::BOTTOM); + _popover_menu->set_parent(*this); + } else { + _popover_menu->remove_all(); + } Gtk::CheckButton *group = nullptr; for (auto const &value : values) { @@ -219,12 +223,9 @@ std::shared_ptr SpinButton::get_popover_menu() auto const item = Gtk::make_managed(); item->set_child(*radio_button); - item->signal_activate().connect( - sigc::bind(sigc::mem_fun(*this, &SpinButton::on_numeric_menu_item_activate), value.first)); - popover_menu->append(*item); + item->signal_activate().connect(sigc::bind(sigc::mem_fun(*this, &SpinButton::on_numeric_menu_item_activate), value.first)); + _popover_menu->append(*item); } - - return popover_menu; } void SpinButton::undo() @@ -232,6 +233,18 @@ void SpinButton::undo() set_value(_on_focus_in_value); } +void SpinButton::_unparentChildren() +{ + if (_popover_menu) { + _popover_menu->unparent(); + } +} + +SpinButton::~SpinButton() +{ + _unparentChildren(); +} + void SpinButton::defocus() { // defocus spinbutton by moving focus to the canvas, unless "stay" is on diff --git a/src/ui/widget/spinbutton.h b/src/ui/widget/spinbutton.h index 1807b7fb31..8b53600eb8 100644 --- a/src/ui/widget/spinbutton.h +++ b/src/ui/widget/spinbutton.h @@ -59,6 +59,8 @@ public: : Gtk::SpinButton(cobject) { _construct(); } + ~SpinButton() override; + void setUnitMenu(UnitMenu* unit_menu) { _unit_menu = unit_menu; }; void addUnitTracker(UnitTracker* ut) { _unit_tracker = ut; }; @@ -86,6 +88,7 @@ private: NumericMenuData _custom_menu_data; bool _custom_popup = false; double _increment = 0.0; // if > 0, key up/down will increment/decrement current value by this amount + std::unique_ptr _popover_menu; void _construct(); @@ -113,7 +116,7 @@ private: unsigned keyval, unsigned keycode, GdkModifierType state); bool on_popup_menu(PopupMenuOptionalClick); - std::shared_ptr get_popover_menu(); + void create_popover_menu(); void on_numeric_menu_item_activate(double value); /** @@ -121,6 +124,8 @@ private: */ void undo(); + void _unparentChildren(); + public: inline void set_defocus_widget(const decltype(_defocus_widget) widget) { _defocus_widget = widget; } inline void set_dont_evaluate(bool flag) { _dont_evaluate = flag; } diff --git a/src/ui/widget/status-bar.cpp b/src/ui/widget/status-bar.cpp index d699ba3231..2980fdd5ae 100644 --- a/src/ui/widget/status-bar.cpp +++ b/src/ui/widget/status-bar.cpp @@ -20,12 +20,12 @@ #include #include #include +#include #include <2geom/point.h> #include <2geom/angle.h> // deg_from_rad #include "desktop.h" #include "ui/builder-utils.h" -#include "ui/menuize.h" #include "ui/pack.h" #include "ui/util.h" #include "ui/widget/canvas.h" @@ -77,7 +77,8 @@ StatusBar::StatusBar() zoom_menu->prepend_item(menu_item); // In reverse order. } - zoom_popover = make_menuized_popover(zoom_menu, *zoom); + zoom_popover = std::make_unique(zoom_menu, Gtk::PopoverMenu::Flags::NESTED); + zoom_popover->set_parent(*zoom); zoom_value->signal_input().connect(sigc::mem_fun(*this, &StatusBar::zoom_input), true); zoom_value->signal_output().connect(sigc::mem_fun(*this, &StatusBar::zoom_output), true); @@ -113,7 +114,8 @@ StatusBar::StatusBar() rotate_menu->prepend_item(menu_item); // In reverse order. } - rotate_popover = make_menuized_popover(rotate_menu, *rotate); + rotate_popover = std::make_unique(rotate_menu, Gtk::PopoverMenu::Flags::NESTED); + rotate_popover->set_parent(*rotate); rotate_value->signal_output().connect(sigc::mem_fun(*this, &StatusBar::rotate_output), true); rotate_value->signal_value_changed().connect(sigc::mem_fun(*this, &StatusBar::rotate_value_changed)); -- GitLab