Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

react-components: support external state for grid #442

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

giannif
Copy link
Collaborator

@giannif giannif commented Apr 22, 2024

allowing for easier data mutations

edit.grid.mov

Copy link

changeset-bot bot commented Apr 22, 2024

🦋 Changeset detected

Latest commit: 69c5a20

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@giphy/react-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@giannif giannif requested a review from sketchybones April 22, 2024 12:14
@giannif giannif force-pushed the react-components/grid-external-state branch from 09fa137 to 9e20c13 Compare April 22, 2024 12:16
prevState: State
) => {
const gutterOffset = gutter * (columns - 1)
const gifWidth = Math.floor((width - gutterOffset) / columns)
if (prevState.gifWidth !== gifWidth) {
return { gifWidth }
}
if (externalGifs && externalGifs !== prevState.gifs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if externalGifs exists, is this ever going to be false? i can't remember if === works with arrays of objects or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sketchybones yes this should be referentially false most of the time, the state in the grid is set to the external state. When you want to update the grid, pass in a new array. I do that in the Story when deleting a gif. If you edit the title, I don't update the array, although I probably should. But since the Gif component rerenders on hover, it grabs the latest title.

I guess if you want it to update instantly, you'd want to clone the array after changing the title:

 const editGif = externalGifs?.find((g) => g.id === gif.id)
  if (editGif) {
      editGif.title = result
      setExternalGifs([...externalGifs])
  }

@giannif giannif marked this pull request as ready for review April 22, 2024 19:37
@giannif giannif merged commit 0f66a98 into master Apr 22, 2024
1 check passed
@giannif giannif deleted the react-components/grid-external-state branch April 22, 2024 19:37
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.

2 participants