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

Style headerlinks for table captions properly #454

Merged
merged 4 commits into from
Dec 16, 2017

Conversation

mitya57
Copy link
Contributor

@mitya57 mitya57 commented Aug 20, 2017

Fixes #437.

Now headerlinks for tables behave the same way like for headers: invisible by default, showing on hover, using Font Awesome.

Copy link
Member

@Blendify Blendify left a comment

Choose a reason for hiding this comment

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

Looks good to me. @agjohnson @ericholscher Would be nice to get more then one approval before committing.

@mitya57
Copy link
Contributor Author

mitya57 commented Sep 28, 2017

Ping?

@Blendify
Copy link
Member

I am doing a dev sprint this weekend will take a look then.

@mitya57
Copy link
Contributor Author

mitya57 commented Sep 28, 2017

Great, thanks. Please also look at #432 if possible.

@Blendify
Copy link
Member

The changes are ok but not perfect, when you hover over the text it shift to the left. Can you try to fix this?

@mitya57
Copy link
Contributor Author

mitya57 commented Sep 29, 2017

@Blendify Please test the latest version.

@mitya57
Copy link
Contributor Author

mitya57 commented Oct 30, 2017

Ping?

@ericholscher
Copy link
Member

Looks like a reasonable change, is there a good example (gif?) of the new change.

@benjaoming
Copy link
Contributor

@Blendify

The changes are ok but not perfect

Subsequent PRs can amend what's an overall improvement

@mitya57 could you post a screenshot to convince the maintainers to press the "merge" button on this and #432 ? :)

@Blendify
Copy link
Member

Blendify commented Nov 9, 2017

I will look at current PRs this weekend

@mitya57
Copy link
Contributor Author

mitya57 commented Nov 10, 2017

Here is a screenshot of what changed.

Before:
before

After:
after

Previously the ¶ symbol was always visible, now it is only visible on hover.

@benjaoming #432 already has a screenshot, see #432 (comment).

&:hover .headerlink
display: inline-block
&:hover .headerlink:after
visibility: visible

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, although I think slightly reducing the link for table captions looks nicer, by adding:

  table > caption .headerlink:after
    font-size: 12px

Current PR:
14px

With font-size: 12px:
12px

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added!

@Blendify Blendify merged commit e9db692 into readthedocs:master Dec 16, 2017
billsacks pushed a commit to ESMCI/sphinx_rtd_theme that referenced this pull request Mar 30, 2020
* Style headerlinks for table captions properly

Fixes readthedocs#437

* Make .headerlink code work with center-aligned elements

* Lowered font size for table headerlink as requested in the review
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.

5 participants