[EuiImage] Allow src as well
#4611
Conversation
| type _EuiImageSrcOrUrl = ExclusiveUnion< | ||
| { | ||
| /** | ||
| * Requires either `src` or `url` but defaults to using `src` if both are provided | ||
| */ | ||
| src: string; | ||
| }, | ||
| { | ||
| url: string; | ||
| } | ||
| >; |
There was a problem hiding this comment.
🤔 Somehow though this doesn't show up as either being required in the props table. Is that a problem? TS will complain at least.
There was a problem hiding this comment.
It's difficult to "project" types into a flattened system like the props tables or the component's propTypes. In this case, you want to allow src or url but not both, and it doesn't make sense to mark both as required as that's equally invalid. I played around with this a little to see if I could trick it to show src as required, but couldn't. We could modify the generated docgen when showing it on the page, however:
src-docs/src/views/image/image_example.js
EuiImage.__docgenInfo.props.src.required = true;
There was a problem hiding this comment.
Oooh I like that idea, and it worked well. Thanks!
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4611/ |
|
Jenkins, test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4611/ |
This
urlprop for EuiImage has tripped me up many times. I constantly try to use the typical HTMLsrcattribute and it takes me several minutes and docs lookup to find my mistake.This PR just creates an exclusive union that requires either
urlorsrcbut since they can also conflict if not using TS, it defaults tosrcsince this feels more standard and aligns to the final<img>tag.Checklist
[ ] Check against all themes for compatibility in both light and dark modes[ ] Checked in mobile[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes