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

Allow keyboard to activate permalink #1162

Merged
merged 4 commits into from Jun 18, 2021

Conversation

jonels-msft
Copy link
Contributor

Fixes #956

In the new behavior, permalinks are hidden but are present for keyboard focus. When you tab into them they appear. They also appear when you mouse over the headers, as before.

The only downside is that we're falling back to using sphinx's ¶ symbol, which doesn't look as nice as fontawesome's chain link icon. However I think that's a price we have to pay for accessibility.

@Blendify
Copy link
Member

The only downside is that we're falling back to using sphinx's ¶ symbol, which doesn't look as nice as fontawesome's chain link icon. However I think that's a price we have to pay for accessibility.

The newer sphinx version allows changing this see https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-html_permalinks_icon I think we should be able to change the default for this from the python side? Not sure though needs investigation...

@jonels-msft
Copy link
Contributor Author

Is this good to merge for now?

@Blendify Blendify requested a review from agjohnson June 14, 2021 23:59
@Blendify
Copy link
Member

We need to see if it's possible to change the default permalink icon from python/sphinx so that there is no regression.

The change would have to be made in https://github.com/readthedocs/sphinx_rtd_theme/blob/master/sphinx_rtd_theme/__init__.py

I think this is possible using app.config.html_permalinks_icon this may be read-only though:
app.config.html_permalinks_icon = "🔗"

@jonels-msft
Copy link
Contributor Author

@Blendify your suggestion worked perfectly in my dev environment. 😄

@extend .fa
&:after
content: "\f0c1"
font-family: FontAwesome
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the rendering does not match how it did previously because the font family should still be FontAwesome.

Also is there a reason behind the font size change and the adjusted margin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After losing the FontAwesome font family, I was adjusting the size and margin to match the original size better.

However, as you suggested, I put FontAwesome back to match the prior style even better. Did have to include a little left margin because the link icon was pressed right against the text otherwise when not in :after content.

Comment on lines 58 to 60
# sphinx emits the permalink icon for headers, so choose one more in keeping with our theme
app.config.html_permalinks_icon = "🔗"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should fix the tests from failing

    if sphinx_version >= (3, 5, 0):
        app.config.html_permalinks_icon = "🔗"
    else:
        app.config.html_add_permalinks = "🔗"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, did that. Also specified the unicode character with "\u...". Hope that kind of string literal is portable across python versions.

@jonels-msft
Copy link
Contributor Author

@Blendify it's ready for another look.

@Blendify Blendify merged commit da86e4d into readthedocs:master Jun 18, 2021
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.

Header permalinks appear on mouse hover, but are inaccessible via keyboard
2 participants