Image updates: remove BG-style, add crossOrigin functionality, allow nullable alt text #2207
+111
−243
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
As noted in #1786 there are a few ways that images could serve customers better. In particular, this diff explicitly aims to fix: #1636 and #2171. (Happy to break this up to focus on each individually but was working on an integration test w/ Twitter)
Solution
While a full rewrite may be necessary to solve all those problems, I think a number of these issues stem from the usage of a pseudo-img-element on top of a background-image design. So if we can remove that, it may help bring focus to the remaining issues. In short, we can provide a sort of interim solution.
The crossOrigin image functionality represents a class of general problem: we cannot use all the native image functionality because we are constrained by ultimately using background image (e.g. see #1542). With privacy sandbox and site isolation coming, CORS authenticated images are necessary for us, this works OK for a normal image, but background-images have no equivalent so it's impossible to accomplish.
Getting rid of the background image also simplifies the DOM and likely fixes things like Windows High Contrast and the ML accessibility issues as well.
CORS / Cross Origin
While the RN API is to use body and headers on the source object, I do not believe this model works with the web. Because we are subject to CORS, and have server-only cookies. We necessarily need to use flags instead of explicit headers.
To solve this, I decided to add a crossOrigin mode flag matching the img element to the source definition.
This change necessitated tweaking ImageLoader for the same reason. This widens the API to take a source object not just a URI on the load method. I believe RN API also misses the mark here. If an image can be loaded with headers...how are you supposed to prefetch and object with headers? I think we should generally assume that all ImageLoader methods should take any valid source type instead of just a string URI.
Object Fit
My interpretation of the existing code is that visually, we could only get the proper resizing behavior by using background-size. This meant that for accessibility and usability (i.e. saving an image), we previously had to put an invisible "Real" img tag over the background-image. Based on that, it seems like using object-fit gives us the best of both worlds and helps simplify the code (we no longer need onLayout callbacks and so on).
The tradeoff is that browser support does not match RNW's current stated support. In particular old Edge and IE. We could get around this by using an Object fit Polyfill. So that's a disucssion.
BREAKING!: Additionally, because it's not replicable via objectFit...this drops support for the
repeat
resizeMode.BREAKING!: Removed Array
Tests
I tried to verify this works in our core image cases on Twitter, but we do not exercise the full functionality of this (i.e. ImageBackground or certain image props like blur or tint).
It appeared that the image related examples looked the same before and after, and that the snapshot changes were reasonable.