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

[Hot Corners] Added hover delay functionality. #5829

Merged
merged 1 commit into from
Oct 26, 2016
Merged

[Hot Corners] Added hover delay functionality. #5829

merged 1 commit into from
Oct 26, 2016

Conversation

Odyseus
Copy link
Contributor

@Odyseus Odyseus commented Oct 22, 2016

This pull request adds the ability to set a hover delay to activate hot corners. Its main purpose is to
avoid accidental hot corners activation. I also added tooltips to the hot corners icons that reflect their current action, as suggested by @anandrkris.

The new Hot Corners settings window would look like this:

hotcornershoverdelayimplemented2

Some technicalities

  • File js/ui/hotCorner.js
    • This PR also fixes some minor white-space inconsistencies on this file (some missing spaces and some extra tabs). But, as I use a code formatter plugin, it also expanded some objects. Let me know if I should revert back those objects expansions.
  • File files/usr/share/cinnamon/cinnamon-settings/modules/cs_hotcorner.py
    • Here I had to add a timer to the on_widget_changed function to avoid repeated triggering from the new SpinButton elements. I tried to add that timer to a new callback for the SpinButton elements, but I wasn't able to pass the correct arguments to the on_widget_changed function. Any advice will be welcome.
    • I used a Gtk.Grid instead of a Gtk.HBox because of its deprecation. Switched to a Gtk.Box as suggested by @collinss.
  • This PR also adds a new string that needs to be translated. I will need advice on how to proceed with this as well.

@anandrkris
Copy link

Hope this gets merged - lot of votes for this :)
#1050

@anandrkris
Copy link

Also, a suggestion - when icon is visible in the corner, can the action be displayed as a tooltip on hover over icon?
Show All workspaces, windows, custom action, etc

@Odyseus
Copy link
Contributor Author

Odyseus commented Oct 24, 2016

Hello, @anandrkris.

I like your suggestion about adding a tooltip to the hot corners icons. I have been doing some tests but, sadly, without success. No matter how or at which point I add the tooltip, it never shows on icon hover.

Here is a shortened version of what I did in case someone has any suggestion:

After adding the import for the tooltip module in the js/ui/hotCorner.js file...

const Tooltips = imports.ui.tooltips;

...I simply added the tooltip to the icon inside the setProperties method like this...

if (this.icon) {
    new Tooltips.Tooltip(this.iconActor, "Tooltip text.");
    this.iconActor.show();
} else
    this.iconActor.hide();

Is it even possible for the hot corners icons to have a tooltip?

@NikoKrause
Copy link
Member

NikoKrause commented Oct 25, 2016

@Odyseus
Try this.

this.tooltip = new Tooltips.Tooltip(this.iconActor, "Tooltip text.");

@Odyseus
Copy link
Contributor Author

Odyseus commented Oct 25, 2016

Hello, @NikoKrause.

I already tried that. The very first thing that I tried was to create the tooltip inside HotCorner._init and then use the set_text() method inside the setProperties method to apply the correct tooltip text. It's pretty straight forward and it works in every applet code that I ever used, except here with the hot corners.

@NikoKrause
Copy link
Member

@Odyseus
Does my suggestion doesn't work?
I tried your code and added this.tooltip =, which you forgot before new
And it showed the Tooltip on icon hover like expected.

@Odyseus
Copy link
Contributor Author

Odyseus commented Oct 25, 2016

@NikoKrause.

Does my suggestion doesn't work?

Like I said, I already tried that. I used the exact code that you posted and it didn't work.

I tried your code and added this.tooltip =, which you forgot before new

I didn't really forget. It isn't needed to store the tooltip in a new variable because the tooltip is already added to the element passed by the first argument of the constructor (here is a functional example that I use in one of my applets). Eighter way, like I stated before, I already added this.tooltip = in front of that line and it still doesn't work for me.

I did my tests in all of my 4 Cinnamon systems with the same results. The tooltip isn't shown when hovering over the hot corner icon.

I will give this rest for now. Maybe someone else can create a different pull request that adds the tooltips to the hot corners icons.

@NikoKrause
Copy link
Member

@Odyseus
Could you check if my branch works for you https://github.com/NikoKrause/Cinnamon/tree/hot-corner-tooltip? I created a commit with your code in there NikoKrause@6099343.

Just replace the hotCorner.js file in /usr/share/cinnamon/js/ui with the file from my branch.

It looks like this on my machine:
bildschirmfoto vom 2016-10-25 20-25-10

@Odyseus
Copy link
Contributor Author

Odyseus commented Oct 25, 2016

@NikoKrause

No, it still isn't working on any of the 4 Cinnamon systems I tried before. But, I tried it on a fith system (an Archlinux system with Cinnamon) and all tooltips work like a charm here! This is very strange!

I hesitate to add to this pull request something that I can't make work reliable on all my systems. If you want to add this feature yourself and send the pull request, here is a Gist with the hotCorner.js file with icons tooltips fully implemented.

@Odyseus
Copy link
Contributor Author

Odyseus commented Oct 25, 2016

@NikoKrause and other code reviewers.

Ignore the previous posts discussing problems while setting the tooltips for the hot corners icons. I had an extension enabled that was interfering with the hot corners code. I disabled it and everything started working as it should.

@anandrkris: If this PR doesn't make it, NikoKrause just send a pull request adding the tooltips to the hot corner icons.

Apologies for the noise.

@collinss
Copy link
Member

collinss commented Oct 25, 2016

@Odyseus A couple notes/suggestions.

  • You don't need to worry about the translations. Putting _() around the string is enough to insure that it will get translated for the next release.
  • Gtk.HBox and Gtk.VBox were deprecated in favor of Gtk.Box, which adds an additional orientation property to make it vertical. Unless you need the extra layout features of Gtk.Grid (which you do not here), I would recommend using Gtk.Box.
  • I would recommend adding the tooltip to the actor as soon as you create it. If you need to make sure the tooltip is not shown under certain circumstances, you can set the text to an empty string later (you don't need to do this if your actor is hidden as it wont show anyway). The way you did it above could lead to multiple tooltips on the same actor, all competing with each other.

Added tooltips to hot corner icons.
Corrected some minor indentation/white space inconsistencies.
Closes #1050
@Odyseus
Copy link
Contributor Author

Odyseus commented Oct 26, 2016

Hello, @collinss.

  • About translations: Got it.
  • About Gtk.Boxs: I have chosen a Gtk.Grid because I couldn't make the Gtk.Box "look good" (basically, I couldn't understand the arguments passed to pack_start). But since you recommended me to use a Gtk.Box, I gave it another try and I successfully made it work.
  • About the tooltips: The code that I posted was just an example. So that someone willing to help me out finding the root of my problem could quickly set up a tooltip with just two lines of code. Thanks to NikoKrause's assistance, I figured that the problem was on several of my systems.

Thanks for your notes/suggestions, collinss.

Pull request updated.

@clefebvre clefebvre merged commit 5adc5b0 into linuxmint:master Oct 26, 2016
@anandrkris
Copy link

Thanks @Odyseus for listening 😄 and again for your efforts. 👏

@Odyseus Odyseus deleted the hotcorners-hover-delay branch November 9, 2016 00:18
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.

6 participants