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 cursor property #1313

Merged
merged 2 commits into from
May 22, 2021
Merged

Conversation

TonCherAmi
Copy link
Contributor

@TonCherAmi TonCherAmi commented May 16, 2021

This pull request adds a new theme property cursor that allows specifying the type of mouse cursor that is set when the widget is hovered. It can be set to either default, pointer, or text:

  • default is the default arrow cursor
  • pointer could be used for clickable areas:
    image
  • text could be used for input fields:
    image

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2021

Codecov Report

Merging #1313 (9e9dd6d) into next (04c006a) will increase coverage by 0.07%.
The diff coverage is 43.18%.

❗ Current head 9e9dd6d differs from pull request most recent head c87a714. Consider uploading reports for the commit c87a714 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1313      +/-   ##
==========================================
+ Coverage   32.98%   33.05%   +0.07%     
==========================================
  Files          42       42              
  Lines       11934    12018      +84     
==========================================
+ Hits         3936     3973      +37     
- Misses       7998     8045      +47     
Impacted Files Coverage Δ
source/view.c 0.00% <0.00%> (ø)
source/xcb.c 0.00% <0.00%> (ø)
source/theme.c 35.49% <57.14%> (+0.37%) ⬆️
lexer/theme-lexer.l 74.68% <100.00%> (+0.25%) ⬆️
lexer/theme-parser.y 63.97% <100.00%> (+0.56%) ⬆️
source/widgets/widget.c 49.84% <100.00%> (+0.15%) ⬆️
test/theme-parser-test.c 98.31% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04c006a...c87a714. Read the comment docs.

Currently only listview element and editbox are supported.
@DaveDavenport
Copy link
Collaborator

Nice.
For me the textbox does not work and the side-bar buttons are not changed.
I wonder if we should make it a property on the widget itself.

@TonCherAmi
Copy link
Contributor Author

TonCherAmi commented May 16, 2021

For me the textbox does not work and the side-bar buttons are not changed.

Huh, interesting.. Could you please share your config?

I wonder if we should make it a property on the widget itself.

Yeah, I guess that would be ideal and I would personally prefer it that way, although I feel like that would take more effort to implement and I'm not sure how useful it would end up being really.

@TonCherAmi
Copy link
Contributor Author

Actually, it doesn't look that difficult to do, I'm going to give it a shot.

@DaveDavenport
Copy link
Collaborator

I ran it with -no-config.
rofi -no-config -show run -sidebar-mode

@TonCherAmi
Copy link
Contributor Author

TonCherAmi commented May 16, 2021

Yep, the sidebar buttons do not work. However the input box seems fine on my system. I think the issue might be that the cursor name for the I-beam on yours is not in the names array. I'll try searching for some more alternative names, they seem to vary from theme to theme so it's a bit difficult to get them right.

@DaveDavenport
Copy link
Collaborator

I never set cursor themes anymore..

(long long long ago I did write a small ui to select them, gcursor, but that got absorbed into gnome settings at some point (and removed again)).

@TonCherAmi
Copy link
Contributor Author

I never set cursor themes anymore..

I think it's not cursor themes specifically, but rather GTK themes in general.

I've mostly got widget properties working, though there's a small problem with the recently added -hover-select option since it relies on the fact that listview_element_motion_notify is called conditionally and we now must call it unconditionally so that the cursor is updated for every widget. How do you feel about removing that option and making selection on hover the default behavior?

@DaveDavenport
Copy link
Collaborator

no direct feeling.. don't use mouse with rofi myself. Will give it some thoughts.

@TonCherAmi
Copy link
Contributor Author

TonCherAmi commented May 16, 2021

no direct feeling.. don't use mouse with rofi myself. Will give it some thoughts.

Nevermind.. We can just check the setting inside the handler, no need to remove it.

@TonCherAmi TonCherAmi changed the title Change mouse cursor on widget hover Add cursor property May 18, 2021
@TonCherAmi
Copy link
Contributor Author

@DaveDavenport I've implemented the new property and it seems to work okay, please let me know if there's anything I've missed. The textbox issue should also probably be fixed now.

@DaveDavenport
Copy link
Collaborator

DaveDavenport commented May 19, 2021

Looks good.

Seems to fully work for me.

@DaveDavenport DaveDavenport merged commit dc28a97 into davatorium:next May 22, 2021
@DaveDavenport
Copy link
Collaborator

thanks.`

@TonCherAmi TonCherAmi deleted the feature/cursor branch May 23, 2021 03:16
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants