-
-
Notifications
You must be signed in to change notification settings - Fork 43
Add dialog to update user info #482
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
Conversation
justify-content: space-between; | ||
} | ||
|
||
.jp-UserInfo-Field > * { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the *
be replaced with a more precise selector? *
has negative performance implications as every DOM element will be evaluated against the path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it with the expected tags.
body: new UserDetailsBody({ | ||
userManager: this._userManager | ||
}), | ||
title: 'User Details' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to translate this title and the error message below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, the widget now receive an IRenderMime.TranslationBundle
.
I don't know if this is the way to do it, or if it better to share the ITranslator
token.
Co-authored-by: Michał Krassowski <[email protected]>
super(); | ||
this._user = user; | ||
this._userManager = props.userManager; | ||
this._translation = props.translation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be named _trans
:
this._translation = props.translation; | |
this._trans = props.translation; |
Because it gets extracted using very simple static analysis, see https://jupyterlab.readthedocs.io/en/latest/extension/internationalization.html#rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I didn't know about that, I'll try to remember it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just gave an example suggestion, it needs to be renamed in a few more places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean renaming also the property name in the options ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed it anyway for better readability, but I thought the name was required when using it only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I thought the name was required when using it only.
Yes, that's correct. My bad I misread the extend of changes in your previous commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😄 took me 5 min reading again, to be sure I was not missing something again.
Co-authored-by: Michał Krassowski <[email protected]>
Restarting CI after |
Thanks for the review @krassowski and @davidbrochart |
This PR allows the user to update its attributes name, display_name, initials, color and avatar_url.
The updatable fields are send by the server, starting from jupyter-server/jupyter_server#1518.
It is backward compatible, the fields will not be updatable if the property is not provided by the server.
record-2025-05-07_07.58.20.webm
Fixes:
TODO