Skip to content

Conversation

roboslone
Copy link

Description

This PR exposes HUD element text via data-text attribute, so it can be targeted by custom CSS. This fixes #3112, here's an example CSS (only hides HUD when navigating links, doesn't break search):

#hud {
  &[data-text="Open link in current tab."] {
    display: none;
  }

  &[data-text="Open link in new tab."] {
    display: none;
  }
}

@philg-dev
Copy link
Contributor

Disclaimer: I'm not sure if my review / feedback means anything - just chiming in with my two cents.

I haven't looked into the functionality of this PR entirely, but from what I can see it would indeed be a very slim solution. However, I'm a little bit concerned to tie such a thing to (exact) user-facing HUD messages.

Custom CSS would become more fragile this way, since it might break very easily if any of the HUD messages get their wording changed for any reason. Also in case the HUD messages ever were to be localized to different languages, it would make it extremely inconvenient to provide a custom Vimium theme to a wider audience when the CSS selectors need to target all different translations of messages.

I personally feel like it would be a lot cleaner to work with something more technical non-user-facing methods, like marker CSS classes to distinguish the different HUD message use-cases on a technical and more structured level. I do of course realize that that's a lot more effort and needs a lot more testing to be implemented properly. The approach could be to use the Vimium command as a CSS class (that on it's own by default doesn't affect the styling) to the HUD. Taking your examples that could look like this:

<div id="hud" class="vimium-ui-component-hidden link-hints-activate-mode"></div>
<div id="hud" class="vimium-ui-component-hidden link-hints-activate-mode-to-open-in-new-tab"></div>

That would at least decouple the CSS selectors from the actual messages. I'm not sure how feasible it would be to add this generically for all commands that result in HUD messages though.

Maybe the maintainers consider your solution totally sufficient though - who knows. On a super actively changing codebase I would consider it a bit "hacky" for sure... But if the HUD messages are considered mature enough to not expect any changes or if they consider potentially breaking custom CSS in rare cases is acceptable, then a simply "hacky" solution may be preferred after all.

As I said - just my thoughts as a "heads-up". Thanks for working on this!

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.

How to hide the "open link in current tab" in the lower right corner?
2 participants