Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Tooltips for masthead links #1380

Merged
merged 1 commit into from
Dec 4, 2017
Merged

Tooltips for masthead links #1380

merged 1 commit into from
Dec 4, 2017

Conversation

maazadeeb
Copy link
Contributor

Fixes #1377

@mmistakes
Copy link
Owner

Shouldn't this be something more like?

<li class="masthead__menu-item">
  <a href="{{ domain }}{{ link.url }}" {% if link.description %}title="{{ link.description }}"{% endif %}>{{ link.title }}</a>
</li>

What you have is somewhat redundant and doesn't really produce HTML that would give the link more context since you're just duping link.title for the anchor name and title attribute`.

For example say you have this:

main:
  - title: "My Masthead Link"
    url: http://whatever.com

You're going to get this...

<li class="masthead__menu-item" title="My Masthead Link">
  <a href="http://whatever.com">My Masthead Link</a>
</li>

Which is kind of pointless since it doesn't add any additional context to the link.

https://www.searchenginejournal.com/how-to-use-link-title-attribute-correctly/

If you use my code:

main:
  - title: "My Masthead Link"
    description: "An awesome website about whatever"
    url: http://whatever.com

You'd get this:

<li class="masthead__menu-item">
  <a href="http://whatever.com" title="An awesome website about whatever">My Masthead Link</a>
</li>

Which I think is better for the user.

@maazadeeb
Copy link
Contributor Author

I added the title attribute on the wrong tag. I meant to add it to the anchor tag actually. Will change it.

I suggested about using a separate key tooltip in the _navigation.yml in the issue, but you suggested that it won't add value. I think I misunderstood maybe? Do you think a key called description would make more sense? I'd be happy to add it if so 😄

@mmistakes
Copy link
Owner

I was referring to how you were setting a default value for title. That is unnecessary as title is already the link name.

description seems more semantic to me. The hover effect isn't really a "tooltip", just something browser's do. Not to mention it means nothing for mobile users since you can hover a link.

@maazadeeb
Copy link
Contributor Author

Fair enough. I've updated my commit. I've updated the docs as well.

@mmistakes
Copy link
Owner

Looks good, thanks @maaz93.

@mmistakes mmistakes merged commit 634d288 into mmistakes:master Dec 4, 2017
kkunapuli pushed a commit to kkunapuli/kkunapuli.github.io that referenced this pull request May 30, 2019
sumeetmondal pushed a commit to sumeetmondal/sumeetmondal.github.io that referenced this pull request Sep 10, 2019
jchwenger pushed a commit to jchwenger/jchwenger.github.io that referenced this pull request May 5, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants