-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add linkText to AlphabetHints #4689
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: master
Are you sure you want to change the base?
Conversation
@philc any feedback? |
I'll look at this soon.
…On Wed, Jun 25, 2025 at 10:43 AM, Jan-Henrik Damaschke < ***@***.*** > wrote:
*itpropro* left a comment (philc/ vimium#4689) (
#4689 (comment) )
@ philc ( https://github.com/philc ) any feedback?
—
Reply to this email directly, view it on GitHub (
#4689 (comment) ) , or unsubscribe
(
https://github.com/notifications/unsubscribe-auth/AAACDFUUMYPZSJYIY436PJT3FLNUZAVCNFSM6AAAAAB36B46Z6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMBVGYZTONJUGU
).
You are receiving this because you were mentioned. Message ID: <philc/vimium/pull/4689/c3005637545
@ github. com>
|
UX comments: Adding "scroll" and "frame" to the hint text can add a lot of clutter to the page and obscure nearby hints. The UX rationale for only having it in filtered hints has been because the clutter in that mode is less of a problem, because you can very quickly type a few characters that you see in the anchor, or in the hint itself, to hide the many other hints on the page. With alphabet hints, you can't quickly filter out the clutter like this. Showing "scroll" and "frame" in filtered hints isn't my favorite because it's not self-explanatory. However, at least that mode has the convention of showing the name of some links in the hints (e.g. for form controls). In alphabet hints, having the word "scroll" appear next to a hint feels even less self-explanatory and more random, because I expect the content of the hints to only be characters that I should type to navigate to the link. I think a change in color might work better, or some other UX. Any thoughts? Feedback about this PR:
![]() Also, I found an adjacent bug which is unrelated to this PR. I tested this change on test_harnesses/iframe.html in the Vimium repo. On that page, after hitting space to toggle the hints, typing S navigates to the first link starting with an S, but instead it should wait for further characters to be typed. Update: I believe I fixed this in 544b0ce. |
content_scripts/link_hints.js
Outdated
@@ -792,18 +792,21 @@ class AlphabetHints { | |||
this.hintKeystrokeQueue = []; | |||
} | |||
|
|||
renderMarker(marker) { | |||
let linkText = marker.linkText; | |||
console.log(linkText); |
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.
Log statement left in.
fillInMarkers(hintMarkers) { | ||
const hintStrings = this.hintStrings(hintMarkers.length); | ||
if (hintMarkers.length != hintStrings.length) { |
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.
Is this safety check no longer needed?
- Added 'scroll-hint-marker' class for frames and scrollable areas in link hints. - Implemented overlap detection and offset adjustments for overlapping hint markers. - Updated CSS for scroll hint markers to improve visibility.
Happy to remove that last bit if it exceeds the scope of this PR. |
This should make it easier to discern scroll/frame markers from links. It's a feature that has been only available with Filter hints enabled. I may be missing something, but I see no reason why it shouldn't be visible when using the default Alphabet hints as well.
#4675