8000 ENH Add alt attribute for thumbnails by lucyleeow · Pull Request #668 · sphinx-gallery/sphinx-gallery · GitHub
[go: up one dir, main page]

Skip to content

ENH Add alt attribute for thumbnails #668

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

Merged
merged 7 commits into from
May 1, 2020

Conversation

lucyleeow
Copy link
Contributor

closes #664

@lucyleeow
Copy link
Contributor Author

@larsoner I don't understand the cause of the test failures. I seems that test_junit and test_rebuild are failing during setup but I don't understand why. tinybuild builds fine when I check.
The error is

ValueError: not enough values to unpack (expected 3, got 2)

but there all 3 output variables (intro, title, cost) are present.

@larsoner
Copy link
Contributor

@lucyleeow
Copy link
Contributor Author

@larsoner the title cannot contain \n so we won't have the same problem as in #669. It can contain non-word characters but will they be a problem?
Also do you think the test is adequate?
And thanks for the tip!

Copy link
Contributor
@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Also do you think the test is adequate?

I find it handy to add a line to test_full.py to check the HTML. Here you can assert that there is alt="..." in the index.html of a gallery containing the title of an example -- it shouldn't be there on master but should be there on this PR

@@ -336,12 +336,13 @@ def generate_dir_rst(src_dir, target_dir, gallery_conf, seen_backrefs):
'generating gallery for %s... ' % build_target_dir,
length=len(sorted_listdir))
for fname in iterator:
intro, cost = generate_file_rst(
print('for {} generate file run'.format(fname))
Copy link
Contributor

Choose a reason for hiding this comment

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

debugging cruft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😂 oh no, my print statement debugging exposed!

@larsoner
Copy link
Contributor

It can contain non-word characters but will they be a problem?

Maybe but we can wait until someone has a problem :)

@codecov-io
Copy link
codecov-io commented Apr 30, 2020

Codecov Report

Merging #668 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
+ Coverage   97.78%   97.79%   +0.01%     
==========================================
  Files          31       31              
  Lines        3247     3266      +19     
==========================================
+ Hits         3175     3194      +19     
  Misses         72       72              
Impacted Files Coverage Δ
sphinx_gallery/backreferences.py 96.29% <100.00%> (ø)
sphinx_gallery/gen_rst.py 97.75% <100.00%> (ø)
sphinx_gallery/tests/test_backreferences.py 100.00% <100.00%> (ø)
sphinx_gallery/tests/test_full.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96b5fed...eb6281a. Read the comment docs.

@lucyleeow
Copy link
Contributor Author

@larsoner added a check in html and rst for index and backref thumbnail alt. Also added a html test to image alts.

@larsoner larsoner merged commit f0b5058 into sphinx-gallery:master May 1, 2020
@larsoner
Copy link
Contributor
larsoner commented May 1, 2020

Thanks @lucyleeow

@lucyleeow
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH More informative 'alt' attribute for thumbnails in index
3 participants
0