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

👈 Disable hover modifier for mobile in HashLink #516

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Jan 8, 2025

This simple PR adds a [@media(hover:hover)] modifier to the class-names for HashLink instances that use hover. The following components are affected:

  • Heading
  • SolutionRenderer
  • References
  • Footnotes
  • Abstract
  • Bibliography
  • Backmatter
  • Keywords

This PR was motivated by #513, but I think the same logic holds -- our mobile site should not lose interactivity. It does change the aesthetics, though:

Screenshots

Screenshot_20250108_100938_Firefox.png

Screenshot_20250108_100934_Firefox.png

Screenshot_20250108_100949_Firefox.png

Forgive the HiDPI images -- I was already playing around with this PR on mobile.

Copy link

changeset-bot bot commented Jan 8, 2025

🦋 Changeset detected

Latest commit: b427284

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages
Name Type
myst-to-react Patch
@myst-theme/providers Patch
@myst-theme/frontmatter Patch
@myst-theme/diagrams Patch
@myst-theme/jupyter Patch
@myst-theme/site Patch
@myst-theme/styles Patch
@myst-theme/common Patch
@myst-theme/icons Patch
@myst-theme/search Patch
@myst-theme/search-minisearch Patch
@myst-theme/book Patch
@myst-theme/article Patch
myst-demo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@agoose77 agoose77 changed the title 👈 Ddisable hover modifier for mobile 👈 Disable hover modifier for mobile in HashLink Jan 8, 2025
@agoose77 agoose77 requested a review from rowanc1 January 8, 2025 11:25
@rowanc1
Copy link
Member

rowanc1 commented Jan 10, 2025

Not sure I totally love the pilcrows showing up everywhere, I think they do take away from the readability?

The footnotes are a loss of functionality for sure. The hashes on titles etc are a much more advanced feature (i.e. you are wanting to share a link to that section from mobile).

Thoughts on not enabling them?

At the least, we should have an additional option in the component or hover: boolean | 'desktop' that enables the current behaviour.

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.

2 participants