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

Fix the broken anchor links in the headings #1020

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

Conversation

zner0L
Copy link
Member

@zner0L zner0L commented Jan 18, 2023

The anchor links I introduced looked hideous and didn't really work well, so this is my second attempt.

@zner0L zner0L requested a review from baltpeter January 18, 2023 15:22
@cypress
Copy link

cypress bot commented Jan 18, 2023

2 failed tests on run #4528 ↗︎

2 77 7 0 Flakiness 0

Details:

Make changes suggested in #1020
Project: datenanfragen/website Commit: c748ca6cbb
Status: Failed Duration: 03:59 💡
Started: Jun 10, 2023 11:01 PM Ended: Jun 10, 2023 11:05 PM
Failed  cypress/integration/use-cases/reactor.spec.js • 2 failed tests

View Output Video

Test Artifacts
Reacting to request responses > Admonition followed by complaint Output Screenshots Video
Reacting to request responses > Free text response Output Screenshots Video

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@mal-tee
Copy link
Member

mal-tee commented Feb 26, 2023

In Firefox 110 I can only click them if I approach them from the text, i.e. hovering from right to left. They won't appear if you come from the left which is kinda odd.

@zner0L
Copy link
Member Author

zner0L commented Feb 27, 2023

They won't appear if you come from the left which is kinda odd.

Either I don't understand what you mean, or I do not have this problem. I haven't updated Firefox to 110, yet, but I doubt that would change anything.

@mal-tee
Copy link
Member

mal-tee commented Feb 27, 2023

I also get this in Chromium 110/Linux and Firefox 110/Linux.
Video: (relevant part at the end) Screencast from 2023-02-27 17-19-43.webm

@baltpeter
Copy link
Member

Same, plus a few other issues. I started working on a fix a few weeks ago but got pretty annoyed (mostly about the alignment) and haven't continued since. :D

@zner0L
Copy link
Member Author

zner0L commented Feb 28, 2023

Ah, that is what you mean! I don't expect that behavior at all…
And yes, the alignment is really annoying, which is why I implemented this in the annoying way I did without breaking the semantics too much.

@zner0L
Copy link
Member Author

zner0L commented Mar 3, 2023

Am I supposed to do anything here? I don’t really think what you describe is expected behavior and it is certainly annoying to implement.

@baltpeter
Copy link
Member

I don’t really think what you describe is expected behavior

I am convinced that it is. Consider two cases:

  • The user hovers over a heading and sees the anchor thingy for the first time. They want to find out what it does but their mouse "slips" from the heading, untoggling the hover state. The intuitive thing to do is to go straight for the thing they wanted to get to anyway (the anchor). That doesn't work.
  • The user already knows the anchor thingy and where it is. So they try to hover it directly. That doesn't work.

And honestly, even as someone who does know how the hover state works and why it behaves the way it does, I find it quite annoying to get to the anchor thingy. And you better be careful not to move the mouse too much after you have activated the hover state. Otherwise you have to start again at the heading. Kind of like one of these annoying flash games from back in the day. :P

1020-before.mp4

Now look at my alternative implementation. The alignment is off (though it really isn't perfect for yours either), but it clearly feels so much more natural:

1020-after.mp4

I do also remember having some accessibility concerns about the markup, but unfortunately I didn't write those down as I had assumed that I would just fix them myself…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants