Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make menu applet resizable with mouse #11665

Closed
wants to merge 5 commits into from
Closed

Make menu applet resizable with mouse #11665

wants to merge 5 commits into from

Conversation

fredcw
Copy link
Contributor

@fredcw fredcw commented May 15, 2023

This is a reworking of PR #9771 The bug causing cinnamon to crash in that PR seems to have disappeared with the cjs rebase to spidermonkey 78 in Mint 20.1. I've also used the grab pointer method instead of the poll timeout method to do the resizing and this is the same code I've used to make Cinnamenu applet resizable for the past 2 years without any problems.

Issues:

In testing the 36 most popular spices themes, the following minor issues were found:

In New-Minty, Numix Cinnamon Transparent and Numix Cinnamon SemiTransparent visual glitches when menu is too tall.
In Graphite Zero, Graphite One and Jade-1 visual glitches when menu is too narrow.

I think these issues would be best fixed in the respective themes.

Edit: PRs to fix above theme issues have been submitted.

When applet can be resized on opposite edges, i.e. when the applet is not in a corner of the workspace, then resizing from either of those edges leads the mouse pointer to not match up with the applet edge when being dragged. This is a minor visual issue that maybe should be fixed in the future.

When applet can be resized on opposite edges, i.e. when the applet is not in a corner of the workspace, then resizing from either of those edges leads the mouse pointer to not match up with the applet edge. This is a minor visual issue that maybe should be fixed in the future.
Add Applet.PopupResizeHandler class to applet to make it resizable.
...to avoid excessive disk writes.

+ remove some commented out lines.
@mtwebster
Copy link
Member

Works well - I noticed one thing, related to another bug unfortunately -

If you try to resize while the app list isn't at the top - basically when you can reproduce the 'invisible menu items above the menu' issue, dragging the top edge to resize will also start a dnd operation on whatever menu item was under the pointer at the time.

Can we block dnd while resizingInProgress == true?

(ps: I revisit the other issue pretty regularly and haven't had any luck fixing it)

@fredcw
Copy link
Contributor Author

fredcw commented May 22, 2023

@mtwebster
Here's the sequence of events:

Mouse button is pressed on menu edge.
dnd.js receives signal and grabs pointer (_onButtonPress() )
resizer receives signal, grabs pointer and begins resizing
button is released
resizer receives signal, ends resizing and ungrabs pointer
dnd.js doesn't receive signal and still thinks button is pressed
dnd.js receives motion event and starts dragging

Unfortunately, there's no global variable or function in dnd.js to detect or change the grab status until dragging begins (after the motion event) which only begins after resizing has stopped.

Not sure what the solution is, I'll think about it. maybe add a global function to dnd.js to cancel all grab events?

@lestcape
Copy link

lestcape commented May 23, 2023

@fredcw There are not any better solution to a problem that avoid to occurs.

So, this is only a problem because the actor is connected to the signal 'button-press-event' in the _Draggable class before is connected to the same signal in the Applet.PopupResizeHandler class. The connection to the _Draggable class occurs in the constructor of this class and happens when you called DND.makeDraggable(this.actor).

Then the solution should be change the order of when that both connections happens and then when you receive a 'button-press-event' in your Applet.PopupResizeHandler class you can try to do a thing like that to avoid the DND activation: this.actor._delegate._draggable.inhibit = true or you can play with this.actor.reactive = false instead.

To change the order of the connection, a general possible way is move the DND.makeDraggable(this.actor) to outside the constructor to a "protected" function (that you will call in the constructor) and override that protected function in your children to do nothing and then after you connected your actor, then call that function using the super class definition.

Another way to change the order of the connection is pass another parameter to the constructor of class like can be make_dragable and only call DND.makeDraggable(this.actor) if the parameter is true (default should be true for compatibility). So, in that way you can control when you want to make an actor draggable. In that way you need to call the constructor with false in the make_dragable then you can defined the actor as resizable and finally you can make the actor draggable, because following that order you ensure the connection order you wanted.

@fredcw
Copy link
Contributor Author

fredcw commented May 23, 2023

@lestcape I though it works in the following way (or something like it): The resizer class connects to the AppletPopupMenu actor while the _dragable class connects to a menu button. Since the menu button is contained within the AppletPopupMenu actor, it will be higher in the z order and will always receive the signal first.

@lestcape
Copy link

lestcape commented May 23, 2023

I was always thinking that it was the same actor, sorry for not fully understanding what was happening. In that case, i recommend the same anyway: "There are not any better solution to a problem that avoid to occurs". Then, you just need to change the type of the event you are capturing in the Applet.PopupResizeHandler class by an event with more precedence and stop the event propagation if your condition for enter in a resize state is satisfy. See for more info:

https://www.gnu.org/software/guile-gnome/docs/clutter/html/ClutterActor.html

Once an actor has been determined to be the source of an event, Clutter will traverse the scene graph from the top-level actor towards the event source, emitting the <"captured-event"> signal on each ancestor until it reaches the source; this phase is also called the capture phase. If the event propagation was not stopped, the graph is walked backwards, from the source actor to the top-level, and the <"event"> signal, along with other event signals if needed, is emitted; this phase is also called the bubble phase. At any point of the signal emission, signal handlers can stop the propagation through the scene graph by returning ‘CLUTTER_EVENT_STOP’; otherwise, they can continue the propagation by returning ‘CLUTTER_EVENT_PROPAGATE’.

I also recommend you to review the condition for enter in a resize state, because you can end disabling fully the draggable property of the menu button actors that way.

Here there are a similar example for a similar situation:
https://github.com/linuxmint/cinnamon/blob/master/js/ui/main.js#L1087-L1092

@mtwebster
Copy link
Member

@fredcw Seems to work well now with your fix.

The only other thing that was mentioned to me was if we could have something in the corner to indicate that it can be resized, like normal windows used to have (don't you have something like this in your menu applet?) Not a big deal if we can't.

Thanks

@fredcw
Copy link
Contributor Author

fredcw commented Jun 4, 2023

@mtwebster I couldn't think of a way to do this in Cinnamenu so I ended up just putting a tip in the options/config dialog that said: "Note/tip: Drag on the menu edge to change the menu size". I could add such a tip in the menu config if you think it's needed.

Now that I know a more about widgets, I suppose it might be possible to place an icon in the corner that would sit above the other widgets and their padding. And it would have to work in all corners depending on where the applet was placed. I suppose it's possible but it would probably take me some time to figure it out.

Edit: And tbh, it's appearance would be highly dependant on the cinnamon theme and it might somewhat spoil the nice neat appearance of the menu. Actually, now I think about it, I suppose you could fit an icon in between the end of the search bar and the menu edge and only have it in top right corner regardless of the menu position on the screen. But then the resizer would have to work on the icon as well so that could get quite complicated.

@mtwebster
Copy link
Member

Ok, I can't think of a simple solution either - I know you could make a particular corner 'different' from the others using the theme, but how to make that difference say 'grab me here', I'm not sure :)

@mtwebster
Copy link
Member

Can you squash this so that there's only two commits:

  1. Support resizing in js/ui/applet.js, dnd.js, etc...
  2. Add support to the menu applet

Thanks

@fredcw
Copy link
Contributor Author

fredcw commented Jun 6, 2023

@mtwebster No problem, I'll have to create a new PR.

@fredcw
Copy link
Contributor Author

fredcw commented Jun 6, 2023

Created new PR #11688

@fredcw fredcw closed this Jun 6, 2023
@fredcw fredcw deleted the menuresize branch June 6, 2023 17:19
@fredcw
Copy link
Contributor Author

fredcw commented Jun 7, 2023

I know you could make a particular corner 'different' from the others using the theme, but how to make that difference say 'grab me here', I'm not sure :)

I didn't think of this option.
With this svg:

menu_resize

and adding:

.menu-background {
  border-image: url("light-assets/menu/menu_resize.svg") 14; }

to cinnamon.css, you could make the Mint-Y light themes look like this:

Untitled

The background color would be hardcoded into the svg so I assume changing the theme bg color would be more difficult in the future.

@Secret-chest
Copy link
Contributor

I think it's not needed. The settings page can include a note about this.

@mtwebster
Copy link
Member

I think it's not needed. The settings page can include a note about this.

That's the kind of thing it's better to avoid. The corner mark is commonly understood to mean 'you can resize this', isn't only discoverable only if they open the menu settings, and doesn't require someone in every supported language to translate it for us.

@fredcw That looks really nice - could it just be a transparent background or would that end up with no color at all there? Otherwise most themes could probably 'inherit the thing from the default one.

@fredcw
Copy link
Contributor Author

fredcw commented Jun 7, 2023

@mtwebster I tried the transparent bg, it just turns the whole menu transparent.

Edit: I think any change to the menu bg in a theme is going to override any default image. I think the feature is somewhat discoverable due the the pointer changing, whether that's enough or not, I'm not sure.

@Secret-chest
Copy link
Contributor

I don't like the visual clutter it creates. Windows' 10 Start Menu was resizeable and it had no corner mark.

@lestcape
Copy link

lestcape commented Jun 7, 2023

I added the mark in configurable menu using a floating actor, but it's also possible duplicate the whole menu actor, and set in the top one actor the transparent background and in the bottom actor the current one with a visible background. In that way people can change the theme without break the mark and if the color of the mark don't match with the theme they can override the svg with one that match.

@JosephMcc
Copy link
Contributor

Keep in mind that if you do a visual indicator like that, you might want to move it based on where the panel is located. Putting it in the upper right is going to seem a bit weird if your menu is in a right side vertical panel. You would also need the placement to account for themes that can use quite large border rounding. So that doesn't end up weird cut off or floating partially out in space.

fredcw added a commit to fredcw/cinnamon that referenced this pull request Jul 20, 2024
Revert a workaround added in commit linuxmint@c5af58b that prevented a dnd bug linuxmint#11665 (comment)

The original bug linuxmint#11123 has now been fixed so this workaround is no longer needed.
mtwebster pushed a commit that referenced this pull request Jul 20, 2024
Revert a workaround added in commit c5af58b that prevented a dnd bug #11665 (comment)

The original bug #11123 has now been fixed so this workaround is no longer needed.
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.

5 participants