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

WIP, ENH: Add alt option to glue figure #394

Closed

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Feb 24, 2022

Closes #211

This tackles the first part of the problem, i.e. adding an alt optional argument to the glue:figure directive, but there's still work to be done to pass through the alt string to the final figure that is embedded in the output. I haven't had success tracing through the various abstractions/transforms that get from a PasteNode within the containing figure to the actual image node that ultimately gets rendered in the output. My sense is the best way to do this is to add a PasteImageNode (or similar) subclass that keeps the alt-text as an attribute, and modify the subsequent transforms etc. to ensure that the alt text is passed all the way through to the final create_render_image step. I'm not sure if this approach is consistent with the current design, so I figured I'd open a WIP PR hoping for someone more knowledgeable to weigh in.

@@ -139,7 +139,7 @@
<inline classes="pasted-text">
undisplayed
inline…
<figure align="default" ids="abc" names="abc">
<figure align="default" ids="abc" names="abc" alt="alt-text for glued figure">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From looking at the output html, it seems to be that the alt should actually be on the image tag two lines below, not the figure. The trick is coming up with a consistent way to pass that information through the various containers/transforms.

@choldgraf
Copy link
Member

Just a quick note that we're likely going to hit some rebase conflicts as @chrisjsewell is nearing completion of #380 - sorry about that!

@rossbar
Copy link
Contributor Author

rossbar commented Feb 24, 2022

Just a quick note that we're likely going to hit some rebase conflicts as @chrisjsewell is nearing completion of #380 - sorry about that!

No worries - these changes really only handle adding an alt option to glue:figure directive; aka "the easy part". My goal was mostly to give this a bump on the priority queue as the lack of support for alt-text is a blocker for adoption in some of our projects due to accessibility concerns.

@chrisjsewell
Copy link
Member

Heya, this was superceded by #403

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.

alt option for glue:figure directive
3 participants