Skip to content

Conversation

hwdsjtu97
Copy link
Contributor

What does this PR do?

Fix #6265

Where should the reviewer start?

WorldMap component

What testing has been done on this PR?

yarn test

How should this be manually tested?

Test with the example in #6265

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?

#6265

Screenshots (if appropriate)

Screen.Recording.2023-01-18.at.8.43.11.PM.mov

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?

Compatible

@hwdsjtu97 hwdsjtu97 force-pushed the hwdsjtu97/worldMap_issue_6265 branch from 73de293 to d4c346c Compare January 19, 2023 05:01
@hwdsjtu97
Copy link
Contributor Author

hwdsjtu97 commented Jan 19, 2023

The issue in #6265 is due to the targets state is not cleaned up before each time the placeProps changes. The consequence is that when placeProps changes to [], the placeElements becomes empty, but targets are still refs to the previous placeElements which are <path>s already unmounted. When the placeProps is set back to places again, the targets won't update because this condition will be false, so targets won't be updated to new placeElements mounted on the page

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.

Nice work, this looks good!
Appreciate the explanation of the changes made 😃

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.

WorldMap component goes off after state update.
3 participants