Skip to content

Conversation

@anesthetice
Copy link
Contributor

fixes #1504

Two things left to iron out:

  1. is "Shaper Extra" the best way to describe what the new popover contains?
  2. padding seems a little short between end of 'Highlighting' and 'Constraints'

@anesthetice
Copy link
Contributor Author

unnamed

<property name="width-request">300</property>
<property name="selection-mode">none</property>
<object class="GtkLabel">
<property name="label" translatable="yes">Shaper Extra</property>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a simple

Shape Configuration (2/2)

and adding a (1/2) on the first page.

Though at that point a next/previous page navigation makes more sense.

Maybe a nagivation view can work inside of the popover ? https://gnome.pages.gitlab.gnome.org/libadwaita/doc/1.7/class.NavigationView.html

@anesthetice
Copy link
Contributor Author

@Doublonmousse Thanks for taking the time to review this PR, I'll try to work on it later today, I think naming the new popup Shape Configuration (2/2) is better than the current Shaper Extra, but still feels a little off, as it wouldn't be clear why these two were made separate...

What do you think about renaming what is currently called Shape Configuration to Shape Styling (and re-using the previous icon that changes depending on whether the Smooth or Rough style is enabled); and instead of Shaper Extra just Shaper Configuration?

As for implementing next/previous page navigation, I'm not in favor, as that would increase the steps needed to change something.

@Doublonmousse
Copy link
Collaborator

@Doublonmousse Thanks for taking the time to review this PR, I'll try to work on it later today, I think naming the new popup Shape Configuration (2/2) is better than the current Shaper Extra, but still feels a little off, as it wouldn't be clear why these two were made separate...

What do you think about renaming what is currently called Shape Configuration to Shape Styling (and re-using the previous icon that changes depending on whether the Smooth or Rough style is enabled); and instead of Shaper Extra just Shaper Configuration?

Then the gear icon would be used for the second panel I guess.
Yes that makes sense and it's also a little more coherent/useful that what we have now (so we know both the shape and the style mode)

As for implementing next/previous page navigation, I'm not in favor, as that would increase the steps needed to change something.

I would have kept the two buttons but added one to switch from one popup to the other. Although after the change of icon it's not needed anymore because you'd know where to click in the first place

@flxzt
Copy link
Owner

flxzt commented Oct 24, 2025

I am skeptical about a separate menu, what about adding a second page to the original menu?

@anesthetice
Copy link
Contributor Author

I am skeptical about a separate menu, what about adding a second page to the original menu?

Would be more elegant but less nice to use in my opinion; could we maybe compromise with a scrollable popover with a fixed height?

@anesthetice
Copy link
Contributor Author

Or alternatively a second page containing only the constraints might work, as that parameter is probably less often changed

@flxzt
Copy link
Owner

flxzt commented Oct 24, 2025

yes, we should definitely re-order all items so that only the least frequently used are on the second page.

@flxzt flxzt marked this pull request as draft November 17, 2025 19:11
@Doublonmousse
Copy link
Collaborator

I wanted to check whether or not I could do a quick test of a popover with two pages using NavigationView but it turns out that using this is an absolute disaster.

  • The HeaderBar is supposed to act differently when inside a NavigationView (here also inside a popover) but it definitely doesn't work leading to ... this
image
  • Even if it did work, it's not great because you could have an arrow to go back from page 2 to page 1 but not the reverse in the modified headerbar

  • The example shows a button to switch from page 1 to 2 added but copying the template doesn't work and will always fail on startup even if I add an id to every single element so ...

I think I could make it work by not using a Header Bar, placing buttons manually and doing the page switch action from the rust code and definitely not from the ui template

@Doublonmousse
Copy link
Collaborator

I've managed to create a basic working version for the 2-paged settings here Doublonmousse@07d2478
Screencast_20251226_205258

If everyone is okay with the design of this, it can be used as a base to transfer the settings over.

@Doublonmousse
Copy link
Collaborator

Doublonmousse commented Dec 31, 2025

From https://gitlab.gnome.org/GNOME/gtk/-/issues/7958#note_2640752 (and linked PR with some utilities for popover https://gitlab.com/inkscape/inkscape/-/merge_requests/7710 where popovers can be made to be scrollable through a utility, and the direction changed to make it fit)

@anesthetice
Copy link
Contributor Author

I've left this one on the back-burner for too long, the 2-paged version looks pretty good, but I'm concerned about how nice it would be to use (e.g., next page button right next to the close popover button → pain on a touchscreen).

When I have the time (soon™), I'll try to see if I can adapt what @Doublonmousse pointed to:

void wrap_in_scrolled_window(Gtk::Popover& popover, int min_height, int min_width) {
    auto child = popover.get_child();
    if (!child || dynamic_cast<Gtk::ScrolledWindow*>(child)) {
        return;
    }

    child->reference();
    popover.unset_child();

    auto scrolled = Gtk::make_managed<Gtk::ScrolledWindow>();
    scrolled->set_child(*child);
    scrolled->set_policy(Gtk::PolicyType::NEVER, Gtk::PolicyType::AUTOMATIC);
    scrolled->set_propagate_natural_height(true);
    scrolled->set_propagate_natural_width(true);
    scrolled->set_min_content_height(min_height);
    scrolled->set_min_content_width(min_width);

    popover.set_child(*scrolled);
    child->unreference();
}

As a scrollable window would likely be nicer to use

@Doublonmousse
Copy link
Collaborator

If we go with the 2 page solution, the hhomogeneous (or is it vhomogeneous) property should be set to true so these close/next buttons don't jump position on the screen (as long as the 2 panes are about the same size) if we go on the 2-page solution.
I can try to put the next/previous buttons on the bottom (but not sure it would be as visible in that case, unless I play a little bit with colors ?).

Maybe making it scrollable is the better solution ?

I think we can play around with both (even if it's to have placeholders around) on touchscreens/pen input to see what works better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shape Tool settings not opening in Windows after building from MSYS2

3 participants