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

Fix duplicate collaborator entries in collaboration panel #422

Merged
merged 3 commits into from
Jan 9, 2025

Conversation

nzinfo
Copy link
Contributor

@nzinfo nzinfo commented Jan 1, 2025

Problem

When the same user connects from multiple browsers, the collaborator panel may show duplicate entries even when they are viewing the same file. This causes unnecessary clutter and React key warnings.

Solution

  • Add deduplication logic in _onAwarenessChanged using a Set to track unique combinations of username and current file
  • Only add a collaborator to the list if their unique key (username + current file) hasn't been seen before
  • Use consistent unique keys in both the awareness change handler and React component rendering

Changes

  • Add collaborators_keys Set to track unique collaborator entries
  • Create unique keys using username and current file path
  • Filter out duplicate entries before adding to collaborators list
  • Update React key prop to use the same unique key format

Testing

  1. Open the same notebook in multiple browser windows with the same user
  2. Verify only one entry shows per unique (username, current file) combination
  3. Check browser console - no more React key warnings

Copy link
Contributor

github-actions bot commented Jan 1, 2025

Binder 👈 Launch a Binder on branch nzinfo/jupyter-collaboration/fix_react_warning

@nzinfo
Copy link
Contributor Author

nzinfo commented Jan 1, 2025

The warning

Warning: Each child in a list should have a unique "key" prop.

Check the render method of CollaboratorsBody. See https://reactjs.org/link/warning-keys for more information.
Collaborator@http://localhost:8888/lab/extensions/@jupyter/collaboration-extension/static/collaboration_lib_index_js.0471f9a278b7578b8646.js:105:76
CollaboratorsBody@http://localhost:8888/lab/extensions/@jupyter/collaboration-extension/static/collaboration_lib_index_js.0471f9a278b7578b8646.js:96:94 react.development.js:209
React 5
printWarning
error
validateExplicitKey
validateChildKeys
createElementWithValidation
CollaboratorsBody collaboratorspanel.js:75

Copy link
Collaborator

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @nzinfo.
How about making collaborators a Map, instead of having the keys in collaborators_keys and the values in collaborators?

@krassowski krassowski added the bug Something isn't working label Jan 7, 2025
@nzinfo
Copy link
Contributor Author

nzinfo commented Jan 8, 2025

@davidbrochart Thank you for the review. I've changed collaborators into a map.

return (
<Collaborator
key={uniqueKey}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it used?

Copy link
Member

Choose a reason for hiding this comment

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

I believe it is needed for React to know which entry is which.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and It's the key that can avoid the warning "Warning: Each child in a list should have a unique "key" prop."

Copy link
Collaborator

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

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

Thanks @nzinfo.

@krassowski krassowski merged commit c956eea into jupyterlab:main Jan 9, 2025
27 checks passed
@nzinfo nzinfo deleted the fix_react_warning branch January 10, 2025 02:29
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

Successfully merging this pull request may close these issues.

3 participants