Skip to content
This repository was archived by the owner on Sep 30, 2025. It is now read-only.

Conversation

laurkim
Copy link
Contributor

@laurkim laurkim commented Apr 11, 2024

WHY are these changes introduced?

Image without optional ref argument was causing type errors.
11-09-t3waz-qe4ny

WHAT is this pull request doing?

Explictly adds ref as an optional prop type on ImageProps

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@laurkim laurkim changed the title [Image] Fix ref type to allow for null [Image] Fix ref type to allow for undefined Apr 11, 2024
@Shopify Shopify deleted a comment from github-actions bot Apr 11, 2024
@Shopify Shopify deleted a comment from github-actions bot Apr 11, 2024
@Shopify Shopify deleted a comment from github-actions bot Apr 11, 2024
@laurkim laurkim added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Apr 11, 2024
@laurkim laurkim changed the title [Image] Fix ref type to allow for undefined [Image] Add optional ref prop type Apr 11, 2024
@Shopify Shopify deleted a comment from github-actions bot Apr 11, 2024
@laurkim
Copy link
Contributor Author

laurkim commented Apr 11, 2024

/snapit

Copy link
Contributor

🫰✨ Thanks @laurkim! Your snapshothas been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240411211213"

@laurkim laurkim marked this pull request as ready for review April 11, 2024 21:32
@laurkim laurkim requested a review from sophschneider April 11, 2024 21:32
Copy link
Contributor

@sophschneider sophschneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TYSM for fixing this!! Not blocking and not sure if this works in your case but you might be able to use forwardRef too

@laurkim
Copy link
Contributor Author

laurkim commented Apr 11, 2024

TYSM for fixing this!! Not blocking and not sure if this works in your case but you might be able to use forwardRef too

For sure, I updated Image in a previous PR to use forwardRef (just doesn't come up in the diff in this PR haha) 👍

@laurkim laurkim merged commit 46d5c63 into main Apr 11, 2024
@laurkim laurkim deleted the lo/fix-image-ref branch April 11, 2024 21:38
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
### WHY are these changes introduced?

Image without optional ref argument was causing type errors.
<img width="1040" alt="11-09-t3waz-qe4ny"
src="https://github.com/Shopify/polaris/assets/26749317/82c54744-641f-48d4-bed2-1388c7d3f13f">

### WHAT is this pull request doing?

Explictly adds `ref` as an optional prop type on `ImageProps`

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🤖Skip Changelog Causes CI to ignore changelog update check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants