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

ShuttleContainer receiving incorrect data-name attribute based on number of re-renders #139

Open
akhilpanchal opened this issue Nov 10, 2021 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@akhilpanchal
Copy link

Hi:

Thanks for this amazing library :).

I am building a shuttle that has filters on both source and target containers.
I am noticing that when the visibility of the Shuttle is toggled (for ex: Shuttle in a multi-step wizard, going back and forth between steps), the data-name attributes are swapped based on how many times the shuttle container is re-rendered (ex: number of characters entered in a container's filter input.)

I have forked the codesandbox for the filter example and updated the implementation to more closely match our use-case.

CSB Link reproducing the issue

Steps to reproduce

  1. Enter a single character in the left (source) shuttle container.
  2. Select an item from the left shuttle container.
  3. Toggle the visibility of the Shuttle by clicking the Toggle button twice.
  4. Select an item from the left shuttle container.
  5. Notice that the item corresponding to the same index is selected in the right (target) shuttle container.
  6. Upon inspecting the ShuttleContainer in the DOM, notice that the data-name attribute of the left shuttle container is now target instead of source.

Following relevant lines in the implementation are causing this behavior:

const id = React.useRef(
SHUTTLE_CONTAINERS_ARRAY[Math.floor(id_int++ % NUMBER_OF_CONTAINERS)] // eslint-disable-line @typescript-eslint/camelcase
);

Upon entering every character in the container's filter Input, the ShuttleContainer is re-rendered causing the id_int to increment. Therefore, if you enter 2 characters in the filter input, then everything works as expected.

Possible Solution

  1. Having a containerCount ref in the Context initialized to 0 ,instead of having a global id_int variable.
...
const { onClick: defaultClickHandler } = useShuttleItemClick({ setShuttleState, shuttleState });

const containerCountRef = React.useRef(0);

return (
    <ShuttleContext.Provider
        value={{ shuttleState, setShuttleState, containerCountRef }}>
...
  1. The ShuttleContainer component can increment the containerCount ref.
...
// mod needed for HMR updates
const id = React.useRef(
    SHUTTLE_CONTAINERS_ARRAY[Math.floor(containerCountRef.current % NUMBER_OF_CONTAINERS)]
);
containerCountRef.current++;
...

Any thoughts or suggestions?
I'll be more than happy to send a PR if this approach makes sense?

@atomicpages atomicpages added the bug Something isn't working label Nov 29, 2021
@atomicpages atomicpages self-assigned this Nov 29, 2021
@atomicpages
Copy link
Owner

👋 thanks for the bug report! Using the context approach works and I feel like this brings up a code smell!

atomicpages added a commit that referenced this issue Nov 29, 2021
- Pass new counter on context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants