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
[MRG + 2] [MAINT] Update to Sphinx-Gallery 0.1.7 #7986
Conversation
Could you please point to a change-log? Thanks |
merge if tests pass, I guess? Not sure what should be reviewed? Can you add an entry to whatsnew? |
I guess having a glance at the gallery and make sure nothing looks off is a good start: https://7552-843222-gh.circle-artifacts.com/0/home/ubuntu/scikit-learn/doc/_build/html/stable/auto_examples/index.html |
The relevant changes of Sphinx-gallery for Scikit-learn from version 0.1.4 to 0.1.6 are Bug Fixes
|
I pushed a fix for the tooltip alignment. Now it left aligns to with the world so that is not anymore overlapping with the sidebar button. For reasons that go beyond my understanding on CSS raising the z-index of the tooltip did not help to put it on top of the sidebar. The highest index is from the bootstrap themes and is 1051, but even putting the tooltip to 2000 did not change anything. |
@@ -209,7 +209,6 @@ a.sphx-glr-code-links[tooltip]:hover:before{ | |||
display: inline-block; | |||
text-align: center; | |||
text-indent: 0; | |||
margin-left: -5em; | |||
margin-left: 0; /* Has to be zero to avoid overlapping with sidebar */ |
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 change a backport of something in sphinx_gallery, or is it a modification specific for scikit-learn?
I am asking because I am worried that if it is specific to scikit-learn, next time that we upgrade, we will break something.
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.
It is a modification done in place for now for scikit learn. Which I believe to be reasonable enough to transfer to sphinx-gallery.
Code tooltips are a recent feature in Sphinx-Gallery from your last PR as you wanted to make their use more explicit for Scipy-lectures code examples.
From a personal point of view, I see little advantage on trying to make them look a bit centered relative to the link and left aligning seems good enough. I was once slightly annoyed they apear bellow the link, because if 2 consecutive lines have function calls, the first tooltip will block the second link and you have to move the mouse around to make them disappear and select the second function link. Then I thought making them inline would be better, and then again I thought that a nested function call I would experience the same issue, so I let it be.
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.
OK. I'll have a look once CircleCI is finished running, and we should either consider porting this back to sphinx gallery, or making the CSS change in a scikit-learn specific file.
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.
OK, looks good. +1 for backport into sphinx-gallery
LGTM |
@Titan-C: for future reference, we have shortcuts to Circle CI artifacts like http://scikit-learn.org/circle?7560/auto_examples I'm not entirely happy with those tooltips. I'm okay to merge this now, despite, but I'd rather:
|
I checked for around 20 examples, it looks fine to me... |
Do we want to wait with this until the sphinx 1.5 bug is fixed? sphinx-gallery/sphinx-gallery#137 |
I would say yes. |
I'm keeping track on what shall get in the next Sphinx-Gallery bug fix release apart from bug fix to Sphinx v1.5 new changes. Concerning code tooltips @jnothman
|
Thanks @Titan-C, I don't really understand why title is breaking with readthedocs, or why that should not be considered a bug on the part of readthedocs (title is, of course, the correct way to do this in Semantic HTML) |
Title does not only break in readthedocs. It breaks on all themes. I retested it last night, the reason is the browser itself will popup the info box with the style defined by the browser. When @GaelVaroquaux tried to put CSS style to this box, the styled box appears first and then a little later the browser pops again its own box. For that reason there is duplication. |
Oh yes, I intended to let the browser handle it. I find the CSS popup
unnecessarily intrusive, personally.
title may not be used in web readers. In the past I think it has been used
by search engines, but that's probable not relevant to our application.
…On 12 December 2016 at 20:50, Óscar Nájera ***@***.***> wrote:
Title does not only break in readthedocs. It breaks on all themes. I
retested it last night, the reason is the browser itself will popup the
info box with the style defined by the browser. When @GaelVaroquaux
<https://github.com/GaelVaroquaux> tried to put CSS style to this box,
the styled box appears first and then a little later the browser pops again
its own box. For that reason there is duplication.
There is of course the option of not to style this at all and let the
browser handle the title attribute.
On the same thread of issue sphinx-gallery/sphinx-gallery#145 (comment)
<sphinx-gallery/sphinx-gallery#145 (comment)>,
Gaël references that title has no effect on web readers for blind people so
there is no true advantage for the title attribute.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7986 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz6_zeFCJ6b0vIjVSk4I2N9UELzc6Xks5rHRkBgaJpZM4LE3zi>
.
|
Tooltip starts with same left alignment as link. This is to avoid tooltip appearing below menu hide sidebar button. The z-index is not working as should putting it on top of the sidebar.
This does not appear to be an umodified 0.1.7 sphinx-gallery version, can you fix this? Running copy_sphinxgallery.sh and diffing I get: diff --git a/doc/sphinxext/sphinx_gallery/_static/gallery.css b/doc/sphinxext/sphinx_gallery/_static/gallery.css
index a7e2890..9de9b97 100644
--- a/doc/sphinxext/sphinx_gallery/_static/gallery.css
+++ b/doc/sphinxext/sphinx_gallery/_static/gallery.css
@@ -195,11 +195,11 @@ a.sphx-glr-code-links:hover{
text-decoration: none;
}
-a.sphx-glr-code-links[tooltip]:hover:before{
+a.sphx-glr-code-links[title]:hover:before{
background: rgba(0,0,0,.8);
border-radius: 5px;
color: white;
- content: attr(tooltip);
+ content: attr(title);
padding: 5px 15px;
position: absolute;
z-index: 98;
diff --git a/doc/sphinxext/sphinx_gallery/docs_resolv.py b/doc/sphinxext/sphinx_gallery/docs_resolv.py
index b2757d2..12346b5 100644
--- a/doc/sphinxext/sphinx_gallery/docs_resolv.py
+++ b/doc/sphinxext/sphinx_gallery/docs_resolv.py
@@ -358,7 +358,7 @@ def _embed_code_links(app, gallery_conf, gallery_dir):
# patterns for replacement
link_pattern = ('<a href="%s" class="sphx-glr-code-links" '
- 'tooltip="Link to documentation for %s">%s</a>')
+ 'title="Link to documentation for %s">%s</a>')
orig_pattern = '<span class="n">%s</span>'
period = '<span class="o">.</span>'
|
Also can you have |
Indeed doing this update I noticed this. Version 0.1.7 has still tooltip and it never goes to title. But I was testing with style at that time too. For some reason I pushed to PyPI a version with title, that is wrong. I'm currently looking how to change the file in PyPI |
Maybe in the enhancements section. Just stay brief and mention that sphinx-gallery has been updated to 0.1.7 and give the PR and your username (look at other entries how to do it with the |
Unless the code you uploaded for 0.1.7 has a big regression, I would not bother too much about it since 0.1.8 is going to be released in not such a long time IIRC. |
the PyPI version of 0.1.7 uses the title CSS atribute. No one has complained, but that gives this problem sphinx-gallery/sphinx-gallery#145 (comment). Yes 0.1.8 shall be out as soon as the relative path PR sphinx-gallery/sphinx-gallery#190 is approved. Sphinx-Gallery 0.1.8 would contain the style changes requested here, of dropping the tooltip in favour of title and letting the browser handle the style of it. |
HTML doc for the CircleCI build can be found here. I looked at the gallery and I can't find any obvious issues. |
Are the download buttons meant to be side-by-side like on sphinx-gallery.readthedocs.io? They're appearing one above the other. |
Hmmm not sure to be perfectly honest. I would still be in favour of merging this PR to be able to use sphinx 1.5 and have CircleCI running fine. We can sort out cosmetic in a later update of sphinx-gallery. |
Are the download buttons meant to be side-by-side like on
sphinx-gallery.readthedocs.io? They're appearing one above the other.
Hmmm not sure to be perfectly honest.
Depends on your screen size (actually on the size of the div that they
are in, which will vary from theme to theme.
I tend to prefer them side-by-side, unless the screen is small.
|
Hmmmm mine should be wide enough, but I agree in any case: merge if we know
no severe issue
…On 17 Jan 2017 9:35 pm, "Gael Varoquaux" ***@***.***> wrote:
> Are the download buttons meant to be side-by-side like on
> sphinx-gallery.readthedocs.io? They're appearing one above the other.
> Hmmm not sure to be perfectly honest.
Depends on your screen size (actually on the size of the div that they
are in, which will vary from theme to theme.
I tend to prefer them side-by-side, unless the screen is small.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7986 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz681YMlRYFqOGBxfsAscegJEYzprJks5rTJlmgaJpZM4LE3zi>
.
|
Are the download buttons meant to be side-by-side like on
sphinx-gallery.readthedocs.io? They're appearing one above the other.
This is totally strange. It is a conflict in the CSS I think. If you
make the screen very narrow they are stacked. Then as you widen the
screen they place themselves side-by-side and as you continue enlarging
the window the get back to be stacked.
|
+1 for merging this now to fix CI on master ASAP and tackle the cosmetic link layout issue in a latter PR. |
Merging this one, thanks a lot @Titan-C ! |
Great. Though we're going to continue to run into CircleCI failures in PRs
until we fix #8058.
…On 18 January 2017 at 09:43, Loïc Estève ***@***.***> wrote:
Merged #7986 <#7986>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7986 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz6wF2le1mi52a1cUHTgGw2NKIKIsxks5rTUQegaJpZM4LE3zi>
.
|
and use latest version of sphinx in CircleCI
and use latest version of sphinx in CircleCI
and use latest version of sphinx in CircleCI
and use latest version of sphinx in CircleCI
and use latest version of sphinx in CircleCI
and use latest version of sphinx in CircleCI
and use latest version of sphinx in CircleCI
latest release of Sphinx-Gallery