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

Right click menu #463

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

franglais125
Copy link
Contributor

I reworked the commits on a new branch to bring it down to two commits, so that we have an easier log history.

Do you think it's ready as it is?

dash.js Outdated
// Popup menu
this.menu = new PopupMenu.PopupMenu(this.actor, alignment, this._position);
this.menu.actor.hide();
this.menu.actor.add_style_class_name('panel-menu');
Copy link
Owner

Choose a reason for hiding this comment

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

Skipping this css class seems to fix the issue with the additional spacing between the menu and the dock when on bottom. Why do we need this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks for spotting it! I don't think it's strictly needed, I might have added when trying different styles. Let me remove that, check and resubmit.

@micheleg
Copy link
Owner

An additional thing: if the popupmenu is open, one can open simultaneusly another popupmenu of the launchers. This is at odds with the behaviour of the others menus, which can only be open one at a time.

@franglais125
Copy link
Contributor Author

As usual, your reviews are very thorough. Congrats!

I pushed two new commits to fix both issues. Thanks.

@micheleg
Copy link
Owner

Thanks for the quick fixes! For the popupmenu, it's still not the same behaviour of the other launchers (which is fist click close the popupmenu, second click open a new one). I guess this is because the applications launchers are children of the actor to which this new popupmenu is attached, and therefore remain active? If so I would't know how to fix this.

@franglais125
Copy link
Contributor Author

Yes, I'd need to wire children to parents... As usual with a lot of features I propose, I try to be minimal to help with maintainability.

Let me give a try, if it gets too hard I'll leave it as it is.

@micheleg
Copy link
Owner

If it gets too messy, I can live with the current status. I'm mainly concern about it hidings additional weird behaviours.

@franglais125
Copy link
Contributor Author

Ok, I'll leave as it is. We should then perhaps open an issue to keep this in mind. Is it ok?

@franglais125 franglais125 force-pushed the right_click_menu_squashed branch from aeba6f6 to eda8607 Compare April 11, 2017 05:57
@franglais125
Copy link
Contributor Author

I just rebased against current master, and added a commit simplyfying the logic in the _rightClickMenu function.

@franglais125 franglais125 force-pushed the right_click_menu_squashed branch from eda8607 to 115e5d3 Compare April 27, 2017 21:03
@franglais125
Copy link
Contributor Author

I rebased this branch on master (after the multi-monitor merge), and added a commit to support multiple docks.

@franglais125 franglais125 force-pushed the right_click_menu_squashed branch from 115e5d3 to 979daa6 Compare May 16, 2017 20:37
@franglais125
Copy link
Contributor Author

If it gets too messy, I can live with the current status. I'm mainly concern about it hidings additional weird behaviours.

I've tried a few times to fix this remaining issue. I wanted to know if you have perhaps an idea on how to go about this.

When the popup menu is shown, all elements on the screen behave properly. It's only the appIcons that are still clickable.

@franglais125 franglais125 force-pushed the right_click_menu_squashed branch 2 times, most recently from f6fc688 to 59aac6e Compare June 16, 2017 21:41
@franglais125
Copy link
Contributor Author

@micheleg I think I've finally found a solution to the right-click menu behavior in [59aac6e].

I tweak the "reactiveness" of the AppIcons with the menu-state-changed signal.

I think this was the only remaining issue here.

@franglais125 franglais125 force-pushed the right_click_menu_squashed branch from 59aac6e to 1574446 Compare August 21, 2017 23:14
@franglais125 franglais125 force-pushed the right_click_menu_squashed branch from 1574446 to eeb066b Compare September 6, 2017 20:01
@franglais125
Copy link
Contributor Author

@micheleg I just had to perform a rebase due to the reworking of the showAppsIcon wrapper to fix a small inconsistency.

@franglais125 franglais125 force-pushed the right_click_menu_squashed branch 2 times, most recently from 84a789b to 017da52 Compare October 5, 2017 17:52
@franglais125 franglais125 force-pushed the right_click_menu_squashed branch from 017da52 to 1b7586e Compare January 6, 2018 04:15
@franglais125
Copy link
Contributor Author

Rebased following the code reorganization!

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.

2 participants