pingcap / website-docs

The next generation of PingCAP Docs. Powered by Gatsby ⚛️.
https://docs.pingcap.com/
MIT License
22 stars 33 forks source link

missing alt text #463

Closed CBID2 closed 5 months ago

CBID2 commented 5 months ago

Describe the bug I did an accessibility test on the website, and the results showed that most of the images/icons were missing alt text. Without them, the content of an image will not be available to screen reader users or when the image is unavailable. This can cause these users to have a harder time navigating the site. To Reproduce N/A

Expected behavior N/A

Screenshots screenshot of accessibility issue

Desktop (please complete the following information):

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

CBID2 commented 5 months ago

Hi @rpaik! :) I raised the issue per your suggestion! :)

dveeden commented 5 months ago

@CBID2 https://github.com/pingcap/docs/issues/4389 might also be somewhat related. This would replace images with ebnf text that than gets rendered. See https://github.com/pingcap/docs/pull/16050/files for an example.

dveeden commented 5 months ago

This is https://wave.webaim.org/ right?

CBID2 commented 5 months ago

@CBID2 https://github.com/pingcap/docs/issues/4389 might also be somewhat related. This would replace images with ebnf text that than gets rendered. See https://github.com/pingcap/docs/pull/16050/files for an example.

Thanks for pointing this out @dveeden! :) So...does this mean I should not do a PR for this one?

dveeden commented 5 months ago

@CBID2 pingcap/docs#4389 might also be somewhat related. This would replace images with ebnf text that than gets rendered. See https://github.com/pingcap/docs/pull/16050/files for an example.

Thanks for pointing this out @dveeden! :) So...does this mean I should not do a PR for this one?

No what I was trying to say is that fixing this issue and the issue I mentioned both help with improving accessibility. Both are needed.

The ebnf diagrams are only replacing a small portion of the total set of images.

CBID2 commented 5 months ago

@CBID2 pingcap/docs#4389 might also be somewhat related. This would replace images with ebnf text that than gets rendered. See https://github.com/pingcap/docs/pull/16050/files for an example.

Thanks for pointing this out @dveeden! :) So...does this mean I should not do a PR for this one?

No what I was trying to say is that fixing this issue and the issue I mentioned both help with improving accessibility. Both are needed.

Ohh thanks for clarifying @dveeden! :)

CBID2 commented 5 months ago

/assign

Yuiham commented 5 months ago

@CBID2 Thanks a lot for reporting this issue.

I found there's no problem with images in markdown. The problem occurs on those image component written in JSX. For example, the alt prop is missing on the following image component.

https://github.com/pingcap/website-docs/blob/00f3cf8070993558195121a4b26ea358c576561f/src/components/MDXComponents/DocHome.tsx#L370

So we need to add an alt prop to the image component and assign alt a value using the label variable. Can you try it this way?

CBID2 commented 5 months ago

@CBID2 Thanks a lot for reporting this issue.

I found there's no problem with images in markdown. The problem occurs on those image component written in JSX.

For example, the alt prop is missing on the following image component.

https://github.com/pingcap/website-docs/blob/00f3cf8070993558195121a4b26ea358c576561f/src/components/MDXComponents/DocHome.tsx#L370

So we need to add an alt prop to the image component and assign alt a value using the label variable. Can you try it this way?

Sure @Yuiham! :) Can you also put the 2024-tidb-docs-dash label on my PR too? For some odd reason, it got removed.

Yuiham commented 5 months ago

@CBID2 Done😀