-
Notifications
You must be signed in to change notification settings - Fork 529
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
Feat: Add Alt text properties and setters #911
base: master
Are you sure you want to change the base?
Conversation
@scanny I tried the snippet you added in the comments of the #512 PR:
And it did not work for me, while this implementation did:
|
Yes, quite right, I left off the "r" on |
Ah ok, i see that working now. Is there any reason behind not wanting to have this be officially supported? I think it would be a nice addition. Are there some edge cases you are worried about? |
It would be great for it to be added to the package, it's just not likely for that to happen this week and I'm not sure when we'll get to it. New features need documentation, tests, and a version deployment to ride along with. Just "working" is not even close to complete enough. So there's more effort here than may be immediately apparent. A local function works today and is vendored in your own code-base so you don't need to wait for someone else to add it or fix or otherwise adjust it later. |
Thats fine, I just wasn't sure what you needed for ACs. I just pushed a test for the getting and setting of the alt text field. Not sure where best to add documentation but if you point i will write it. Thanks @scanny |
@pa-t This looks pretty good and I think I'm going to be in here soon adding some other things so I can probably bundle it in; I've made it a shortlist item. On the docs, the key thing is the analysis document, which you can think of as a PPEP (python-pptx enhancement proposal). Here one of the ones we have so far: In this particular case, I think you would just be adding some things to this existing page since this is really a small extension to available shape properties. But the key things are:
All that gives us enough to determine whether we've got the design right. So generally this would be the first commit and we can discuss and refine it before investing in an implementation. This analysis is separately mergeable once resolved. You're welcome to make an initial implemention just for exploratory purposes, in this case that's a great idea. It's just that we might change our mind about aspects of the design or behavior so you wouldn't want to put in more work than you'd be willing to do over if the approach changes, as it often does. The user documentation is derived from the docstrings, so that comes later and is usually pretty automatic. |
Say @pa-t, on the documentation bit, not sure we'd need a lot, but can you help me understand when alt-text is used and what it's for? A couple questions:
And anything else you can think of. I'm trying to place it in a user context to understand how the feature would be used, when it would be relevant, etc. |
hey @scanny
I think the best way to frame this feature is through the lens of enabling more accessibility support. Screen readers are the main use case, only other use i can think of is in the case of remote resources not loading and defaulting to alt-text Do you want me to write the docs for this? |
I'd be thinking we want to resolve to a short blurb that goes in the docstring and leave it at that. Probably no more than 50 words and maybe less, but as long as it needs to be to set folks' expectations. I think starting big and then whittling down to a concise summary (like we're doing) is the right approach. The thing I don't see yet is the default behavior. Does this get anything by default? Otherwise I'm thinking it going to be empty 99.99% of the time. If PowerPoint automatically provides default values when (at least certain) shapes are created, what are those? Like is it the filename of the image in the case of a Picture for example? Or would the user expect these to be blank unless they happened to know the author had explicitly set them? |
Also, what is this attribute called in the MS API? A search on "powerpoint vba get alt text" is a good place to start. |
No nothing is set by default, it is just left empty. The user has to set this value. Found this doc page where it is just called |
@scanny how does something like this read: The |
Adds ability to read and set alt text. Wanted to be able to leverage this library to add in alt text for screen readers for the visually impaired
Property methods taken from @cray2015 pr: #512