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

Add a user package to represent the current connected user #11443

Merged
merged 24 commits into from
Dec 9, 2021

Conversation

hbcarlos
Copy link
Member

This PR was started in #11180, I decided to close the other one to separate every feature in a different PR.

This PR adds two new packages, "@jupyterlab/user", and "@jupyterlab/user-extension". These packages introduce two new plugins:

ICurrentUser:

Provides a representation of the connected user. The idea is that some extensions will require the ICurrentUser to read the information (docprovider to use this identity in RTC, or an extension to add comments in a notebook, etc.). At the moment, all the users are anonymous since we don't have any identity provider, but the idea is that other extensions can swap the default implementation of the current user to fetch the information from a third-party identity provider.
This interface includes the property role. For now, this property is useless since any other package supports RBAC to limit the actions that users can do. There is already an issue open to discuss what roles should we have.

IUserMenu:

Provides a new menu added to the JupyterLab toolbar with the current user's name and icon. The idea behind this widget is to provides a lumino menu where extensions can add different buttons to actions related to the user (Login/Logout buttons, settings, etc.).

References

solves #10965

Code changes

Adds two new package, "@jupyterlab/user", and "@jupyterlab/user-extension".

User-facing changes

Now users will see his name and icon at the right side of the toolbar.

image

Backwards-incompatible changes

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@jtpio
Copy link
Member

jtpio commented Nov 12, 2021

Thanks for starting this!

Adds two new package, "@jupyterlab/user", and "@jupyterlab/user-extension".

I think some CI checks are expected to fail because these packages are not published to npm yet.

@hbcarlos
Copy link
Member Author

hbcarlos commented Nov 13, 2021

Thanks Jeremy!! I updated the images of the UI test to include the new menu.
Okay, I realized that I need to change the general UI test because every time is run, creates a new random username and color.

@hbcarlos hbcarlos marked this pull request as ready for review November 13, 2021 15:14
@fcollonval fcollonval added this to the 4.0 milestone Nov 15, 2021
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @hbcarlos

please find some suggestions for this PR.

galata/test/jupyterlab/general.test.ts Outdated Show resolved Hide resolved
packages/metapackage/package.json Outdated Show resolved Hide resolved
packages/user-extension/src/index.ts Outdated Show resolved Hide resolved
packages/user-extension/src/index.ts Outdated Show resolved Hide resolved
packages/user/src/components.tsx Outdated Show resolved Hide resolved
packages/user-extension/src/index.ts Outdated Show resolved Hide resolved
packages/user-extension/src/index.ts Outdated Show resolved Hide resolved
packages/user-extension/src/index.ts Outdated Show resolved Hide resolved
packages/user/style/menu.css Outdated Show resolved Hide resolved
packages/user/test/user.spec.ts Outdated Show resolved Hide resolved
@jtpio
Copy link
Member

jtpio commented Nov 22, 2021

Thanks @hbcarlos for the continued effort on this 👍

Now users will see his name and icon at the right side of the toolbar.

Posting here a summary of a discussion with Carlos on another channel.

At the moment the plugin adds the name and icon to the top area, even if the user does not plan to use any of the RTC features to collaborate with others. For example a single user only using JupyterLab to do some work locally.

Since this was the subject of a discussion at the time when we added the Simple Interface toggle (see #9100, #9105 and #9296 for more context), I would like to raise a concern about having this name / icon indicator visible at all time in the top area.

This is also related to some user feedback (for example from jupyter/notebook#6210) about the JupyterLab UI having too many distractions, and a new visual indicator in the top area might add some more noise.

Possible alternatives we discussed could be to place to indicator in the bottom left corner (similar to VS Code):

image

image

Not sure the left area handler is currently able to pull items down. Otherwise it could also be a good opportunity to add that feature (in a separate PR).

@hbcarlos
Copy link
Member Author

Thanks for commenting on the user menu @jtpio. I'll remove the widget and we can tackle that on another PR.

@fcollonval
Copy link
Member

fcollonval commented Nov 23, 2021

I fixed the integrity test; in fact in addition to the new user packages, the docprovider package style needed to be skipped because it has now style (due to its dependency on the new user package).

Regarding the very good point raised by @jtpio, my understanding from the JLab team meeting was that no UI elements should be displayed if JupyterLab was not executed with collaborative edition. This means that the collaboration state needs to be stored in the pageconfig object.

Note regarding retrolab: it does not have sidebars. So the current proposition à la Google doc sounds better for me.

@hbcarlos
Copy link
Member Author

I fixed the integrity test; in fact in addition to the new user packages, the docprovider package style needed to be skipped because it has now style (due to its dependency on the new user package).

Thanks, @fcollonval! I did not realize about the docprovider.

This means that the collaboration state needs to be stored in the pageconfig object.

That's actually a good idea. I can remove the menu if the collaborative flag is false.

@fcollonval
Copy link
Member

That's actually a good idea. I can remove the menu if the collaborative flag is false.

👍

@jtpio
Copy link
Member

jtpio commented Nov 23, 2021

Note regarding retrolab: it does not have sidebars. So the current proposition à la Google doc sounds better for me.

Since the plugin adding the UI element to the top area is quite small, RetroLab could also define its own if needed.

But if we keep the current one behind the collaborative flag, then it should work automatically in RetroLab (might have to tweak the ranks there though)

@trungleduc
Copy link
Member

👍 on the ideal of @jtpio, we can place the user widget to the bottom right corner and use it to indicate both the user information and the list of connected users.

@hbcarlos
Copy link
Member Author

hbcarlos commented Dec 9, 2021

Follow-up PR would be fine.

Thanks, @krassowski!
I'm already working on it, I'll add on a follow-up PR as soon as it is done.

@hbcarlos
Copy link
Member Author

hbcarlos commented Dec 9, 2021

I'm pushing one more commit to tag the ICurrentUser token and interface as experimental.
This will give us more flexibility for future changes.

@jtpio
Copy link
Member

jtpio commented Dec 9, 2021

Thanks!

Let's merge and address the remaining items in follow-up issues and PRs.

@jtpio jtpio merged commit 969fa5d into jupyterlab:master Dec 9, 2021
@jtpio
Copy link
Member

jtpio commented Dec 9, 2021

Making a new alpha release now.

@martinRenou
Copy link
Member

@meeseeksdev please backport to 3.6.x

@lumberbot-app
Copy link

lumberbot-app bot commented Oct 13, 2022

Awww, sorry martinRenou you do not seem to be allowed to do that, please ask a repository maintainer.

@martinRenou
Copy link
Member

Oh come on, be nice Meeseeks

martinRenou pushed a commit to martinRenou/jupyterlab that referenced this pull request Oct 13, 2022
…b#11443)

* Adds a new package for the user information

* Read user info from IUser when opening a new document

* Adds a user menu

* Remove anonymous names from docprovider and runs integrity and eslint:typed

* Small fix menu

* Rename tokens ID to match the token name

* Adds new tokens to the documentation

* Review

* Remove components

* Adds tests

* Create the user menu only if collaborative flag is true

* Revert changes on UI tests

* Adds tests to the user menu

* Review

* Integrity

* Small issue

* Store user in local storage, remove StateDB and run integrity

* Remove 'id', 'name', and adds 'givenName', 'familyName', and 'initials'

* Update packages as part of the rebase

* Removes role, and solves an issue with names on the editor

* Change tests user package

* Makes colors private, removes public methods from IUserMenu

* Package integrity updates

* Tag ICurrentUser token and interface as experimental
martinRenou pushed a commit to martinRenou/jupyterlab that referenced this pull request Oct 13, 2022
…b#11443)

* Adds a new package for the user information

* Read user info from IUser when opening a new document

* Adds a user menu

* Remove anonymous names from docprovider and runs integrity and eslint:typed

* Small fix menu

* Rename tokens ID to match the token name

* Adds new tokens to the documentation

* Review

* Remove components

* Adds tests

* Create the user menu only if collaborative flag is true

* Revert changes on UI tests

* Adds tests to the user menu

* Review

* Integrity

* Small issue

* Store user in local storage, remove StateDB and run integrity

* Remove 'id', 'name', and adds 'givenName', 'familyName', and 'initials'

* Update packages as part of the rebase

* Removes role, and solves an issue with names on the editor

* Change tests user package

* Makes colors private, removes public methods from IUserMenu

* Package integrity updates

* Tag ICurrentUser token and interface as experimental
@fcollonval
Copy link
Member

@meeseeksdev please backport to 3.6.x

@lumberbot-app
Copy link

lumberbot-app bot commented Oct 13, 2022

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 3.6.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 969fa5d674931a89322d60c7e8c5bf80752f6cdd
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #11443: Add a user package to represent the current connected user'
  1. Push to a named branch:
git push YOURFORK 3.6.x:auto-backport-of-pr-11443-on-3.6.x
  1. Create a PR against branch 3.6.x, I would have named this PR:

"Backport PR #11443 on branch 3.6.x (Add a user package to represent the current connected user)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

@martinRenou
Copy link
Member

Thanks for trying @fcollonval, but I also realized there would be conflicts. I made a big PR manually backporting all the IUser and collaboration PRs #13242 (still draft for now)

martinRenou pushed a commit to martinRenou/jupyterlab that referenced this pull request Oct 17, 2022
…b#11443)

* Adds a new package for the user information

* Read user info from IUser when opening a new document

* Adds a user menu

* Remove anonymous names from docprovider and runs integrity and eslint:typed

* Small fix menu

* Rename tokens ID to match the token name

* Adds new tokens to the documentation

* Review

* Remove components

* Adds tests

* Create the user menu only if collaborative flag is true

* Revert changes on UI tests

* Adds tests to the user menu

* Review

* Integrity

* Small issue

* Store user in local storage, remove StateDB and run integrity

* Remove 'id', 'name', and adds 'givenName', 'familyName', and 'initials'

* Update packages as part of the rebase

* Removes role, and solves an issue with names on the editor

* Change tests user package

* Makes colors private, removes public methods from IUserMenu

* Package integrity updates

* Tag ICurrentUser token and interface as experimental
hbcarlos added a commit to hbcarlos/jupyterlab that referenced this pull request Jan 29, 2023
…b#11443)

* Adds a new package for the user information

* Read user info from IUser when opening a new document

* Adds a user menu

* Remove anonymous names from docprovider and runs integrity and eslint:typed

* Small fix menu

* Rename tokens ID to match the token name

* Adds new tokens to the documentation

* Review

* Remove components

* Adds tests

* Create the user menu only if collaborative flag is true

* Revert changes on UI tests

* Adds tests to the user menu

* Review

* Integrity

* Small issue

* Store user in local storage, remove StateDB and run integrity

* Remove 'id', 'name', and adds 'givenName', 'familyName', and 'initials'

* Update packages as part of the rebase

* Removes role, and solves an issue with names on the editor

* Change tests user package

* Makes colors private, removes public methods from IUserMenu

* Package integrity updates

* Tag ICurrentUser token and interface as experimental
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet