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

Add support for actions on hover #2466

Closed
wants to merge 6 commits into from
Closed

Add support for actions on hover #2466

wants to merge 6 commits into from

Conversation

Astrono2
Copy link

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Other: Replace this with a description of the type of this PR

Description

This will allow for actions to be triggered on hover. Hover actions will work like toggle actions, meaning that the module is responsible for keeping track of its state, the action call only notifies of a change in state.

Related Issues & Documents

Closes #1064

Documentation (check all applicable)

  • This PR requires changes to the Wiki documentation (describe the changes)
  • This PR requires changes to the documentation inside the git repo (please add them to the PR).
  • Does not require documentation changes

The feature is far for complete, so I can't predict the documentation changes. But there will certainly be some and I'll update this before marking it as ready for review.

Added HOVER to the mousebtn enum and the logic in bar.cpp to handle hover actions.
In the mouse motion event handler, there's now a check for actions with the HOVER mask.
Bar now keeps track of the last action string detected in the hover check. If the action detected is different from the las action, both actions are called. Otherwise, nothing happens. This means that hover works like a toggle.
I'm currently bipassing the check for whether to listen to motion events or not with a '|| true'. This will be replaced by a better alternative later on.
Also added hover action support to custom/script.
Copy link
Member

@patrick96 patrick96 left a comment

Choose a reason for hiding this comment

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

The approach looks solid.

I am right now thinking about how users would use this. I think one of the main use-cases would be to spawn some sort of popup to display some information. Basically launching a program that has to stop running once the mouse leaves the hover region or the bar again.

This would mean we would need to detect and respond to the mouse leaving the hover region or bar and send some signal to the process. This could get quite complicated.
What do you think of this?

What kind of use-cases did you have in mind?

src/components/bar.cpp Outdated Show resolved Hide resolved
if (!string_util::compare(m_hover_act, m_last_hover_act)) {
m_log.trace("bar: Switched hover action from '%s' to '%s'", m_last_hover_act, m_hover_act);
if(m_last_hover_act != "") {
m_sig.emit(button_press{m_last_hover_act});
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to trigger the previous action again?

Copy link
Author

Choose a reason for hiding this comment

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

Because hover works as a toggle. A hover action lets the module know that its hover state changed, it's up to the module to keep track of that state. I think that's the simplest way to handle it.

Copy link
Author

Choose a reason for hiding this comment

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

Speaking of, I'm having a big problem: you can't emit button press signals twice in the same method, the controller swallows the second one. I'm sure there's a simple fix, but I'm not very familiar with the codebase yet.
@patrick96 could you give me a hand? Is there a simple way to enqueue the second signal? Or a worse alternative, emit one and then stall until the next iteration of the eventqueue worker loop?
For reference, this isn't a problem when going from hovering a module to hovering nothing, because there is no second action to be called, but it is when having two modules with hover next to each other.

Copy link
Member

Choose a reason for hiding this comment

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

Because hover works as a toggle. A hover action lets the module know that its hover state changed, it's up to the module to keep track of that state. I think that's the simplest way to handle it.

I see. In that case, do you think it would make more sense to have separate "mouse enter" and "mouse leave" events?

Speaking of, I'm having a big problem: you can't emit button press signals twice in the same method, the controller swallows the second one.

Yeah, the enqueueing of commands isn't great. Currently we store the command in m_inputdata and swallow all other commands as long as it has not been handled and the variable reset.
This could probably easily be replaced with a queue of strings where we place incoming commands at the end and process_inputdata takes one command (or all of them) from the front and processes them.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that's a good idea, but I think it's a bit outside of my scope.
I'll open an issue requesting that rework and put this on hold.

I see. In that case, do you think it would make more sense to have separate "mouse enter" and "mouse leave" events?

I'm not sure. The problem with that would be that then the action would change. It can be taken into consideration, but I don't think the extra complexity it would need is worth it.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure. The problem with that would be that then the action would change.

I don't think this would be very complex to solve. Right now we emit button presses when the current hover action changes. One for the new one and one for the old one. We would do the same thing but store both the current "mouse enter" action (so that we don't run on every mouse move, only the first time) and the current "mouse leave" action (so that we can detect when we leave a region and have to run the old leave action).

I'm just thinking about the usability. It's one thing if it's just the module itself that's keeping state, that's really easy. But as soon as the user wants to run something only while in the hover region, keeping state becomes very cumbersome.

src/components/bar.cpp Outdated Show resolved Hide resolved
@Astrono2
Copy link
Author

The approach looks solid.

I am right now thinking about how users would use this. I think one of the main use-cases would be to spawn some sort of popup to display some information. Basically launching a program that has to stop running once the mouse leaves the hover region or the bar again.

This would mean we would need to detect and respond to the mouse leaving the hover region or bar and send some signal to the process. This could get quite complicated.
What do you think of this?

What kind of use-cases did you have in mind?

This already detects the mouse leaving the hover region. I'm gonna add detecting leaving the bar next.
When the mouse leaves the hover region, it detects that it's hovering a new action (including no action). It then sends a hover signal to the previously hovered action. The only problem with this would be if two modules with the exact same action string for hover were next to each other.

I have in mind tooltips mostly. But also something like mapping #date.toggle to hover instead of click-left.

@Astrono2
Copy link
Author

Oh and speaking of detecting when hovering a different action than before, I added two m_log.trace for debugging.
m_log.trace("bar: Detected hover action %s", m_hover_act); I think is too annoying to keep.
But m_log.trace("bar: Switched hover action from '%s' to '%s'", m_last_hover_act, m_hover_act); could be left in.

This includes always checking mouse enter/leave events.
Also changed an unnecessary !string_util::compare for a !=
@patrick96
Copy link
Member

The only problem with this would be if two modules with the exact same action string for hover were next to each other.

I think we could then count this as one big hover region.

@Astrono2
Copy link
Author

I think we could then count this as one big hover region.

Luckily for me, that's the current behavior lol

@patrick96
Copy link
Member

Just FYI, I won't be able to work on polybar for a few weeks.

@Astrono2 Astrono2 closed this by deleting the head repository Nov 20, 2022
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.

[Feature request] Allow actions on hover
3 participants