-
-
Notifications
You must be signed in to change notification settings - Fork 414
Don't add cursor: pointer to vars when variable highlighting is inactive
#4870
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for the PR @twiss. Just to clear my understanding: With var highlighting on, I think we agree we should continue showing |
|
I think #3765 would be a great addition, yeah, though I think it's in principle orthogonal from this change indeed in the sense that that change should be made in src/core/highlight-vars.js (by removing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the cursor: pointer CSS property from var elements that don't have interactive functionality in exported documents, improving the user experience by avoiding misleading cursor behavior.
- Restricts the positioning CSS rule to only apply to
varelements with adata-typeattribute - Removes the
cursor: pointerproperty entirely from the datatype CSS module - Relies on the existing var.css.js module to handle cursor styling for interactive variable highlighting
| // prettier-ignore | ||
| export default css` | ||
| var { | ||
| var[data-type] { |
Copilot
AI
Aug 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSS selector change from var to var[data-type] may create inconsistency if the position: relative property is needed for all var elements, not just those with data-type attributes. Consider whether this positioning change could affect the layout of var elements without data-type attributes.
| var[data-type] { | |
| var { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't needed, the position: relative is only to help position the position: absolute ::before and ::after pseudoelements below.
✅ Deploy Preview for respec-pr ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
In exported documents, variable highlighting is inactive, and clicking on
vars doesn't do anything. So, I think we shouldn't setcursor: pointeron them.This seems to possibly have been a mistake and only intended for variables with a data type hover bubble, but even there, clicking on them doesn't do anything so setting
cursor: pointeron them still doesn't particularly make sense, IMHO.For non-exported documents when highlighting is active, src/styles/var.css.js will still set
cursor: pointer, so this shouldn't break that case.