feat: Add alt prop to Image component by gabrieldonadel · Pull Request #34550 · react/react-native

gabrieldonadel · 2022-08-31T21:47:16Z

Summary

This adds the alt prop to the Image component as requested on #34424. Using this new alt prop enables the accessibility prop and passes down the alt text to accessibilityLabel. This PR also updates RNTester ImageExample in order to facilitate the manual QA.

Open questions

~~On web alt text is displayed on the page if the image can't be loaded for some reason, should we implement this same behavior if the Image component fails to load source?~~ Not for now

Changelog

[General] [Added] - Add alt prop to Image component

Test Plan

Open the RNTester app and navigate to the Image page
Test the alt prop through the Accessibility Label via alt prop section, this can be tested either by enabling Voice Over if you're using a real device or through the Accessibility Inspector if you're using a simulator

Screen.Recording.2022-08-31.at.18.41.15.mov

analysis-bot · 2022-08-31T22:10:40Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,728,076	+509
android	hermes	armeabi-v7a	7,130,955	+502
android	hermes	x86	8,034,620	+492
android	hermes	x86_64	8,007,908	+501
android	jsc	arm64-v8a	9,597,281	+363
android	jsc	armeabi-v7a	8,363,583	+354
android	jsc	x86	9,541,506	+355
android	jsc	x86_64	10,133,905	+358

Base commit: 8cdc9e7
Branch: main

analysis-bot · 2022-08-31T23:22:13Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 8cdc9e7
Branch: main

ryancat · 2022-09-01T00:30:09Z

On web alt text is displayed on the page if the image can't be loaded for some reason, should we implement this same behavior if the Image component fails to load source?

It's tricky, but I think we should do it.

Pros:

There's no hover with mobile screens, so the alt really can only be rendered into words, instead of using a placeholder image with hovered text
Matching web spec

Cons:

Mobile screen is much smaller, and we need to avoid super long alt that breaks user experiences unexpectedly (How web handles super long alt?).

necolas · 2022-09-01T01:52:30Z

At this stage, I don't think we need to display the alt text if the image doesn't load. Often on web we try to prevent broken images breaking layout by hiding fallback text anyway. This is what RNWeb does today. We can always revisit this decision in the future once people start using it and if a case is made.

necolas · 2022-09-06T18:49:37Z

Ping @cipolleschi or @jacdebug to import

gabrieldonadel · 2022-09-08T16:10:24Z

Could someone please import this? @cipolleschi @NickGerleman @jacdebug @ryancat

facebook-github-bot · 2022-09-19T12:30:23Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

lunaleaps

Can we also update the TS types?

gabrieldonadel · 2022-09-19T21:58:53Z

Can we also update the TS types?

Sure, I've just updated it

necolas · 2022-09-20T00:25:05Z

The TS types are already out of sync with the Flow types, as they don't have props from the other merged Web PRs

lunaleaps · 2022-09-20T06:26:05Z

The TS types are already out of sync with the Flow types, as they don't have props from the other merged Web PRs

I'm actively updating them to catch up (like the accessibility, id ones) and for going forward, we can keep them up to date.

Summary: While working on #34550 I noticed that a couple of words inside the RNTester package were misspelled, this gave me the idea to test other files as well using the VS Code extension [Code Spell Checker](https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker) which unveiled quite a few other typos. ## Changelog [Internal] [Fixed] - Fix RNTester typos Pull Request resolved: #34757 Test Plan: Shouldn't require much testing as this is just fixing some typos inside the RNTester package. Reviewed By: dmytrorykun Differential Revision: D39722889 Pulled By: cortinico fbshipit-source-id: a575ab8337586c5fe2d68ce73d2aae27d24a6384

cipolleschi · 2022-09-22T11:04:07Z

Hi @gabrieldonadel could you please fix the linting warning? After that I'll reimport this. ;)

gabrieldonadel · 2022-09-22T11:53:10Z

Hi @gabrieldonadel could you please fix the linting warning? After that I'll reimport this. ;)

Sure thing, you're talking about these lint warnings that the github-actions bot commented, right?

gabrieldonadel · 2022-09-22T18:31:35Z

Hi @gabrieldonadel could you please fix the linting warning? After that I'll reimport this. ;)

@cipolleschi I've pushed a commit fixing all warnings

facebook-github-bot · 2022-09-23T13:11:35Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2022-09-26T09:12:12Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2022-09-27T09:34:15Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

react-native-bot · 2022-09-27T11:10:04Z

This pull request was successfully merged by @gabrieldonadel in 71fda5e.

^{When will my fix make it into a release? | Upcoming Releases}

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Aug 31, 2022

react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Aug 31, 2022

ryancat reviewed Sep 1, 2022
View reviewed changes

Comment threadLibraries/Image/Image.android.js Outdated

gabrieldonadel requested a review from ryancat September 1, 2022 01:56

necolas mentioned this pull request Sep 1, 2022
[w3c] ☂️ Web Props (Part 1) umbrella issue #34424
Closed
70 tasks

gabrieldonadel requested review from necolas and removed request for ryancat September 2, 2022 02:10

gabrieldonadel force-pushed the feat/add-alt-to-image branch from bbd6b3e to d6e24d9Compare September 5, 2022 17:25

gabrieldonadel requested review from ryancat and removed request for necolas September 5, 2022 18:41

gabrieldonadel force-pushed the feat/add-alt-to-image branch 2 times, most recently from 34db210 to 2490538Compare September 8, 2022 15:40

gabrieldonadel force-pushed the feat/add-alt-to-image branch 2 times, most recently from 53027c8 to f551198Compare September 9, 2022 19:02

gabrieldonadel requested review from necolas and removed request for ryancat September 16, 2022 16:19

necolas approved these changes Sep 16, 2022
View reviewed changes

lunaleaps suggested changes Sep 19, 2022
View reviewed changes

gabrieldonadel force-pushed the feat/add-alt-to-image branch from f551198 to 44da077Compare September 19, 2022 21:56

gabrieldonadel requested a review from lunaleaps September 19, 2022 21:59

lunaleaps approved these changes Sep 19, 2022
View reviewed changes

gabrieldonadel mentioned this pull request Sep 21, 2022
chore: Fix RNTester typos #34757
Closed

cipolleschi approved these changes Sep 26, 2022
View reviewed changes

gabrieldonadel added 5 commits September 26, 2022 16:28

feat: Add alt prop to Image component
71920d0

chore: Update Accessibility Label ImageExample
3fb3624

chore: Invert accessibilityLabel precedence
53c31dd

chore: Add alt prop to TS definition
0c3db7a

fix: Eslint warnings inside ImageExample
989d8d8

gabrieldonadel force-pushed the feat/add-alt-to-image branch from c467683 to 989d8d8Compare September 26, 2022 19:30

facebook-github-bot closed this in 71fda5eSep 27, 2022

react-native-bot added the Merged This PR has been merged. label Sep 27, 2022

gabrieldonadel deleted the feat/add-alt-to-image branch September 27, 2022 12:06

Uh oh!

Conversation

gabrieldonadel commented Aug 31, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Open questions

Changelog

Test Plan

Uh oh!

analysis-bot commented Aug 31, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

analysis-bot commented Aug 31, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryancat commented Sep 1, 2022

Uh oh!

Uh oh!

necolas commented Sep 1, 2022

Uh oh!

necolas commented Sep 6, 2022

Uh oh!

gabrieldonadel commented Sep 8, 2022

Uh oh!

facebook-github-bot commented Sep 19, 2022

Uh oh!

lunaleaps left a comment

Choose a reason for hiding this comment

Uh oh!

gabrieldonadel commented Sep 19, 2022

Uh oh!

necolas commented Sep 20, 2022

Uh oh!

lunaleaps commented Sep 20, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cipolleschi commented Sep 22, 2022

Uh oh!

gabrieldonadel commented Sep 22, 2022

Uh oh!

gabrieldonadel commented Sep 22, 2022

Uh oh!

facebook-github-bot commented Sep 23, 2022

Uh oh!

facebook-github-bot commented Sep 26, 2022

Uh oh!

facebook-github-bot commented Sep 27, 2022

Uh oh!

react-native-bot commented Sep 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gabrieldonadel commented Aug 31, 2022•
edited
Loading

analysis-bot commented Aug 31, 2022•
edited
Loading

analysis-bot commented Aug 31, 2022•
edited
Loading

lunaleaps commented Sep 20, 2022•
edited
Loading