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 hover actions #2868

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

Add hover actions #2868

wants to merge 17 commits into from

Conversation

Astrono2
Copy link

@Astrono2 Astrono2 commented Nov 21, 2022

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

Modules can now register actions to trigger when the mouse enters or leaves an area with two new mousbtn entries: HOVER_START and HOVER_END.

Script module now accepts hover-start = ... and hover-end = ... commands.

Bar config now accepts disable-hover-checking = (true|false) setting to disable checking for hover. This also factors into whether the bar subscribes to mouse motion and mouse enter/leave events.

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 Wiki documentation for the script module should include the two new hover_start and hover_end actions. Also the formatting section on inline actions should update to say that action numbers 9 and 10 are now for hover start and hover end.

The section on bar settings should also add the new disable-hover-checking key and clarify that it's intended use is to improve performance if the user doesn't want actions.

@Astrono2 Astrono2 marked this pull request as ready for review November 21, 2022 12:30
@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #2868 (4185c2d) into master (d5498c8) will decrease coverage by 0.01%.
The diff coverage is 19.04%.

❗ Current head 4185c2d differs from pull request most recent head ea59f96. Consider uploading reports for the commit ea59f96 to get more accurate results

@@            Coverage Diff             @@
##           master    #2868      +/-   ##
==========================================
- Coverage   13.17%   13.15%   -0.02%     
==========================================
  Files         162      162              
  Lines       11509    11539      +30     
==========================================
+ Hits         1516     1518       +2     
- Misses       9993    10021      +28     
Flag Coverage Δ
unittests 13.15% <19.04%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/components/types.hpp 11.11% <ø> (ø)
src/components/bar.cpp 0.00% <0.00%> (ø)
src/modules/script.cpp 0.00% <0.00%> (ø)
src/tags/parser.cpp 93.82% <100.00%> (+0.05%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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 code looks good. I very much prefer this solution to the original toggle implementation (even if it may sometimes be a bit more verbose for users). If there is a demand for it, we can just add a hover-toggle option which either just produces both hover-start and hover-end tags or is its own mouse "button" that just does both. The latter is probably preferable because it keeps it "mouse config option maps to a single action tag" property and it can be used when the hover-toggle option doesn't exist.

I'm interested in what kind of use cases you have found and played around with for this feature.

I have been playing around with the date module and it does look really nice:

label = %{A9:#date.toggle:}%{A10:#date.toggle:}%date%%{A9 A10}

No mouse hover:

before

Mouse hovering over date module:

after

It also makes for a very cool automatically opening and closing menu module:

format = %{A9:#menu.open: A10:#menu.close:}<label-toggle><menu>%{A9 A10}

Also, something to note is that when the bar content changes and the mouse is in a new hover region, the hover code is not triggered until the mouse moves again. I don't necessarily find this a limitation, I just wanted to point this out so we're all aware of it. It also has the nice side-effect of preventing the infinite repetition of the hover start action triggering a bar update, which changes the bar content, which triggers the hover end action, which changes the bar content back and starts the whole thing from the beginning again.

And congrats on your new job!! 🥳

src/components/bar.cpp Show resolved Hide resolved
@@ -444,7 +448,7 @@ vector<exception_test> parse_error_test = {
{"%{O0ptx}", exc::OFFSET},
{"%{O0a}", exc::OFFSET},
{"%{A2:cmd:cmd:}", exc::TAG_END},
{"%{A9}", exc::BTN},
{"%{A11}", exc::BTN},
Copy link
Member

Choose a reason for hiding this comment

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

Maybe actually use mousebtn::BTN_COUNT in this string directly so that we don't have to manually update it.

Copy link
Author

Choose a reason for hiding this comment

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

Would this be a good way of doing it?

std::string invalid_mouse_btn = std::to_string(static_cast<int>(mousebtn::BTN_COUNT));

...

{"%{A" + invalid_mouse_btn + "}", exc::BTN},

src/tags/parser.cpp Outdated Show resolved Hide resolved
src/components/bar.cpp Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@shasherazi
Copy link

Bump?

@Astrono2
Copy link
Author

Astrono2 commented Feb 9, 2023

Bump?

I'm waiting for @patrick96 to get back to me on the requested changes 😅. If he approves of the way I want to implement what he asked for I can add those changes in an instant.

@shasherazi
Copy link

@patrick96? Please approve I've been waiting for ages for this feature please please 😩

@Infinixius
Copy link

@patrick96 +1

@xoores
Copy link

xoores commented Oct 25, 2023

Thanks a lot for this!

I just yoinked it and I will extend this a little bit so more modules can have alternate version on hover 😁 👍

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
5 participants