Skip to content

Conversation

Lukazovic
Copy link
Contributor

What does this PR do?

Image's onLoad prop is not provided to an Img tag

Where should the reviewer start?

src/js/components/Image/Image.js

What testing has been done on this PR?

yarn test

How should this be manually tested?

  1. You can change the Image storybook story in src/js/components/Image/stories/Simple.js to:
export const Simple = () => (
  <Image
    src="//v2.grommet.io/assets/IMG_4245.jpg"
    onLoad={() => console.log('onLoad')}
  />
);
  1. Run the Storybook,
  2. Go to the Image Simple story
  3. Open the browser console to check if the onLoad message appears

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6202

Screenshots (if appropriate)

Do the grommet docs need to be updated?

No

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

Backwards compatible

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good!

@jcfilben jcfilben requested a review from ericsoderberghp July 1, 2022 17:08
@ericsoderberghp ericsoderberghp merged commit f8c7358 into grommet:master Jul 1, 2022
@Lukazovic Lukazovic deleted the Fix-image-onLoad-not-being-called branch July 1, 2022 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants