cloudinary / cloudinary-react

React components that utilize Cloudinary functionality
MIT License
502 stars 219 forks source link

cloudinary-react: Rendered img tag missing alt attribute #161

Closed joeldbirch closed 4 years ago

joeldbirch commented 4 years ago

Bug report for Cloudinary React SDK

Before proceeding, please update to latest version and test if the issue persists Done

Describe the bug in a sentence or two.

Rendered img tag is missing required alt attribute. I have made sure I have a value set for the image's "Description (alt)" field in Cloudinary's UI, but it does not get output to the front-end.

Issue Type (Can be multiple)

[ ] Build - Can’t install or import the SDK [ ] Babel - Babel errors or cross browser issues [ ] UI/Performance - Display or performance issues [X] Behaviour - Functions aren’t working as expected (Such as generate URL) [ ] Documentation - Inconsistency between the docs and behaviour [ ] Incorrect Types - For typescript users who are having problems with our d.ts files [ ] Other (Specify)

Steps to reproduce

  1. Upload an image
  2. Set its "Description (alt)" field to some text
  3. Copy its public_id for use in the next step
  4. import {Image} from 'cloudinary-react' /* ... later ... */ <Image publicId={public_id} />
  5. Use a browser's web inspector to note that the rendered img tag is missing the required alt attribute.

Error screenshots

Screen Shot 2020-05-25 at 6 37 39 pm

Browsers (if issue relates to UI, else ignore)

[ ] Chrome [ ] Firefox [ ] Safari [ ] Other (Specify) [X] All

Versions and Libraries (fill in the version numbers)

React Cloudinary SDK - 1.5.0 React - 16.13.1 Create-React-App - N/A

joeldbirch commented 4 years ago

I'm not seeing a test for this here.

eyalktCloudinary commented 4 years ago

Hey @joeldbirch , Though it can be confusing, the "Description (alt)" that can be set in the Cloudinary UI, is strictly part of the image metadata and not related to the HTML "alt" attribute.

You can, however, retrieve the value of the this context metadata and then set it as "alt" text manually. For example, the resource method of the Admin API returns the context metadata of the relevant asset - https://cloudinary.com/documentation/admin_api#get_the_details_of_a_single_resource

joeldbirch commented 4 years ago

Thanks for the quick reply. I respectfully disagree that this issue is resolved as the point is that an img tag must have an alt attribute. Even if purely presentational, the alt attribute must still exist but be blank, eg: alt=“”.

Also, given the declarative nature of the React wrapper, it seems strange to require users to retrieve the alt text imperatively. It removes the convenience if such required functionality needs to be implemented manually.

On 25 May 2020, at 10:46 pm, Eyal Katz Talmon notifications@github.com wrote:

 Closed #161.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

eyalktCloudinary commented 4 years ago

@joeldbirch Thanks for the feedback. I apologize for not mentioning - you can add any HTML attribute to the image element created by Cloudinary's SDKs. In React for example -

<Image publicId={public_id} alt={alt_text}></Image>

As for retrieval of the "Description (alt)" context metadata of an image, which I mentioned earlier - I agree that it is not relevant here - this was just an example of how it could be possible. Automatically creating the image tag with the alt attribute set to the value configured for "Description (alt)" in the Cloudinary UI is not supported.

joeldbirch commented 4 years ago

I see, thank you for clarifying. It appears that I'll have to be content with ensuring I can pass both public_id and alt_text to the Image component from this library.

To correct my earlier statement: I was wrong that alt is "required", per se. From MDN (emphasis mine):

The above example shows usage of the element:

The src attribute is required, and contains the path to the image you want to embed. The alt attribute holds a text description of the image, which isn't mandatory but is incredibly useful for accessibility — screen readers read this description out to their users so they know what the image means. Alt text is also displayed on the page if the image can't be loaded for some reason: for example, network errors, content blocking, or linkrot.

In light of this I suppose having alt at least default to an empty string could be considered opinionated and unnecessary for inclusion in this library (although it is the opinion I still hold).

Thanks again for your time and consideration.

eyalktCloudinary commented 4 years ago

Sure, thank you for reaching out! In my opinion, having an informative alt is indeed important, and should be set. It can also help with SEO.