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

RFC 97: Alt Text Capabilities #97

Merged
merged 7 commits into from
Aug 9, 2024

Conversation

Chiemezuo
Copy link
Contributor

@Chiemezuo Chiemezuo commented Jun 4, 2024

View the rendered RFC with graphs

In its current state, Wagtail’s alt text functionality presents a significant accessibility challenge. Its approach to alt text relies heavily on the implementation of developers who use Wagtail, with the only default mechanism being that when alt text is not provided, the title of the image is used as the alt text. The problem with this is that image titles are also defaulted to what the file is saved as, and site editors are not mandated to change the titles from this default. Research by Wagtail’s accessibility team has shown that many sites have not attempted to customize the alt text mechanism, instead, relying on the default to file names. Much too often, this leads to a very poor experience for screen-reader users.

Additionally, the developers who create their own ways of enforcing alt text do so in a way that does not allow alt text to be tailored to the context in which the image is used. This is likely due to a general lack of awareness of the value that contextual alt text provides to users. Therefore with images in Wagtail, we have a lot of sites not adhering to Web Accessibility Standards, despite the developers behind them thinking otherwise.

This RFC proposes a solution to improve the overall accessibility of Wagtail sites by expanding the alt text capabilities provided out of the box, thereby enforcing good practices. It approaches this by adding supported mechanisms for handling alt texts in their most common use-case scenarios; such as in StreamField Image blocks, StructBlocks, and by providing helpful image descriptions in the AbstractImage model.

Achieving these objectives, will by extension, provide a way for contextual alt text to be supported for images, especially when they appear in more than one place in a Wagtail website.


Here are some of the things we will ideally not be doing:
1. We will not be doing away with the title field for the AbstractImage model. This is because titles will still be used to identify specific images, especially among a list or point of reference.
2. We will not fall back to the title in the absence of an alt text. This is because we want to enforce alternative text, and a fallback (to another fallback) would be counter-productive to this effort.
Copy link
Member

Choose a reason for hiding this comment

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

Question: Should we consider an opt-in to the legacy behaviour to not break existing Wagtail installations' behaviour? E.g. a Wagtail setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, images that have already defaulted to their titles in the absence of an alt text ideally should not be affected. The change will affect images from after this new behaviour has been introduced.

@Stormheg What are your thoughts on this?


### 2. Adding `image_description` field to `AbstractImage`

The next major action is to add an `image_description` field to the AbstractImage class for a high-level overview of the image. It will be used as the alt text inside the image listing view and inside image picker dialogs. With this in place, the behaviour of the gallery in the admin interface would be such that stored images are described. This would be important to screen-reader users using the Wagtail admin, and it would be a small step in mentally preparing editors to be thorough at the point of image usage. However, with backward compatibility in mind, this new field will default to a blank string for existing images.
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion (readibility): I think it might it make read better to call out the specific goal of this change explicitly in this RFC, e.g. improving accessibility of the Wagtail admin interface for the administrative users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at it now, this does improve readability,

text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Show resolved Hide resolved
@lb-
Copy link
Member

lb- commented Jun 9, 2024

Thank you @Chiemezuo - I had a read and cannot see anything I would like to add. Thank you so much for putting this together.

It may be good to work through whether this RFC replaces the other similar RFC #51

@Chiemezuo
Copy link
Contributor Author

@lb- Yes, it does replace RFC #51
Although they have a similar goal, the approaches are quite different, and there have been a lot of changes to Wagtail since then. My mentors and I felt a new RFC might be better.

@lb-
Copy link
Member

lb- commented Jun 9, 2024

Thanks @Chiemezuo I think that makes sense, can you please update the RFC with a link to the other one and a note about this one superseding that one. Additionally, a comment on the other RFC.

Amazing work here, excited to see this be outworked.

@Chiemezuo
Copy link
Contributor Author

Thank you @lb-
I'll do just this!

@thibaudcolas thibaudcolas changed the title Alt Text Capabilities RFC RFC #97: Alt Text Capabilities Jun 13, 2024
@thibaudcolas thibaudcolas changed the title RFC #97: Alt Text Capabilities RFC 97: Alt Text Capabilities Jun 13, 2024
@gasman
Copy link
Contributor

gasman commented Jun 20, 2024

One nice-to-have feature that isn't mentioned here is the ability to switch an existing ImageChooserBlock to an ImageBlock and have it just work with no additional data migrations. I think this should be achievable quite easily by patching ImageBlock.to_python (or possibly normalize, or both) to check if the incoming value is an integer - which would mean that it's receiving data that previously belonged to an ImageChooserBlock - and handle that by looking up the image with that ID, along with its default alt text.

@vsalvino
Copy link

Regarding fallback - I think a fallback is needed to support existing sites.

  • Add new alt text field to Image model.
  • Use a getter method to generate the alt text - first on alt text field, then on title.
  • The advantage of the getter method is that the dev can override it if necessary to fit their site.
  • Add accessibility warning to the image edit view if the alt text is empty.

@Stormheg
Copy link
Member

Stormheg commented Jul 1, 2024

Hey @vsalvino, I think I can answer your above points.

We're planning to write upgrade considerations explaining how existing sites can make use of the image_description field by doing a data migration from a custom alt text field (a common custom implementation we've seen) to the new image_description field. This data migration is something developers have to write and run themselves.

Use a getter method to generate the alt text - first on alt text field, then on title.
The advantage of the getter method is that the dev can override it if necessary to fit their site.

We explicitly don't want to fallback to the title as it is often a garbage filename and something we very much want to move away from as a default in Wagtail. Yes, this is breaking change, but one we believe needs to be made.

If one does want to retain the implicit title-as-alt-text implementation as fallback, I would suggest overriding AbstractImage's default_alt_text method which - while not explicitly documented - is often already used to implement custom alt text. I suppose this would function as the getter you are describing. Perhaps we should considering documenting it as a workaround to restore the old behavior, though we don't really want to encourage this for the above reason.

Add accessibility warning to the image edit view if the alt text is empty.

Yep! This is already on our radar as part of the implementation 👍


Do you feel this addresses your concerns?

@vsalvino
Copy link

vsalvino commented Jul 1, 2024

Thanks, definitely document default_alt_text as part of the upgrade considerations!

My perspective - almost all sites are using the stock Image model. And most sites have thousands or tens of thousands of images.
So if we make the upgrade difficult on them in the name of accessibility, they will simply never upgrade. We definitely need to provide specific upgrade instructions for sites that currently have NO alt-text.

@Stormheg
Copy link
Member

Stormheg commented Jul 1, 2024

Thanks for the reply @vsalvino. Discussion about this is good, though I'm not sure I fully understand your perspective and concern.

The data migration I mentioned (and that you appear to be perceiving as something that would impede upgrading?) would only be recommended (but not mandated) for sites using a custom image model with an alt text field. The intent of this recommendation being they can transfer any alt text they already have for existing images and adopt the new Wagtail defaults over their own custom implementation.

Developers of a site using the stock image model ('sites that have no alt text') won't have to perform any upgrade steps at all. We're not requiring the image description for existing images be filled during the upgrade if that part is unclear.

The resulting change in behavior will be that instead of images being rendered with a garbage filename alt attribute there won't be an alt attribute at all. If a visualization helps, this is what the HTML diff would look like.

-<img source="DSC_01234.jpeg" alt="DSC_01234.jpeg">
+<img source="DSC_01234.jpeg">

To reiterate the reason behind this (in different words): we consider no alt text to be equivalent to bad alt text.

The difference being that missing alt text is easier for accessibility checker tools to flag. I don't feel this will be a behavioral change that non-accessibility focused developers would really care about.

@vsalvino
Copy link

vsalvino commented Jul 1, 2024

That sounds great - thanks for the detailed reply.

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

It seems to me like there is a large gap between what the RFC covers and what I see discussed in the comments to the RFC / what we’ve discussed in meetings. @Chiemezuo could you work to resolve that gap? For the RFC to be meaningfully approved, it needs to be a clear representation of our plans for implementation.

Here are particular aspects of the proposal that must be crystal-clear:

  1. How any proposed changes affect stock image rendering ({% image my_img %}), in particular what is the proposed generation of the alt attribute on an image rendition. A diagram might help.
  2. What specific breaking changes are foreseen, and how they will be mitigated.

And I would also like to see explicitly covered:

  1. Whether there are any foreseen changes to images added to content via rich text (where there are already contextual alt text fields)
  2. How the proposed changes would affect the "bulk upload" view, as we had discussed specific challenges there in recent meetings.

text/097-alt-text-capabilities.md Show resolved Hide resolved
text/097-alt-text-capabilities.md Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Show resolved Hide resolved
@thibaudcolas
Copy link
Member

For anyone interested in this, we had internal discussions about the proposal and have devised changes to respond to my feedback. @Chiemezuo will update the RFC, I’d recommend holding off from further reviews in the meantime.

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

Minor point of formatting: I’d recommend breaking up your paragraphs into multiple lines (see for example Semantic Line Breaks). This will make it easier for reviewers to comment on a more specific point of your proposal.

text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
@wimfeijen
Copy link

wimfeijen commented Jul 18, 2024

Hi, for us it is very important that we can set an alt text per image in the imaging library. For example: "Voedselbosbouw team", "Voedselbosbouw logo", "portrait of Kaat", "people planting trees". And in Dutch as well, so this would probably need to be multi-lingual.

We have no need of changing alt texts based on the page the image is shown, "people planting trees" would still be a good description for us.

[updated comment for clarification]

@Chiemezuo
Copy link
Contributor Author

Hi, for us it is very important that we can set an alt text per image in the imaging library. For example: "Voedselbosbouw team", "Voedselbosbouw logo", "portrait of Kaat", "people planting trees". And in Dutch as well, so this would probably need to be multi-lingual.

We have no need of changing descriptions based on the page the image is shown, "people planting trees" would still be a good description for us.

Hi @wimfeijen
Image descriptions will not change across pages or in situations where the image is used multiple times on the same page. They will remain fixed. That's where the new ImageBlock comes in. It'll allow you set alt text per image (even when the same image is used multiple times).

For example, say I have image "🖼️", and I have pages "1️⃣" and "2️⃣". At the point of uploading the image, the description will be set, and it will be fixed.
The image 🖼️ can be used in pages 1️⃣ and 2️⃣. In both pages, it will have the same description, but the alt texts will be treated as unique. This is also the same behaviour for when the same image is used multiple times on the same page.

@Chiemezuo Chiemezuo requested a review from thibaudcolas July 18, 2024 09:50
text/097-alt-text-capabilities.md Show resolved Hide resolved
text/097-alt-text-capabilities.md Show resolved Hide resolved
Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

All looking great. I’ve suggested a few editorial changes, that shouldn’t affect the functionality we’re discussing here.

text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Show resolved Hide resolved
@thibaudcolas
Copy link
Member

Marking this as Final Comment Period as we now have two approvals from the core team (me and Storm). Even at this stage I’d feel more comfortable if we got more feedback / explicit approval on this. If anyone is interested do feel free to comment, and we can delay merging of the RFC as needed.

Copy link
Member

@lb- lb- left a comment

Choose a reason for hiding this comment

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

Thank you for preparing this proposal @Chiemezuo - it's really well thought out and I'm all for approving this.

I have added a few small suggested changes, just grammar items though.

text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
text/097-alt-text-capabilities.md Outdated Show resolved Hide resolved
Co-authored-by: Thibaud Colas <[email protected]>
Co-authored-by: LB (Ben Johnston) <[email protected]>
@thibaudcolas
Copy link
Member

Thank you @lb- for the final review 😌. With no further feedback, looks like it’s time to merge this. Nice one @Chiemezuo! 🚀

@thibaudcolas thibaudcolas merged commit ec2b8c4 into wagtail:main Aug 9, 2024
@Chiemezuo
Copy link
Contributor Author

Thank you @thibaudcolas 🥳

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

Successfully merging this pull request may close these issues.

8 participants