-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Users item toolbar #379
Users item toolbar #379
Conversation
@@ -0,0 +1,192 @@ | |||
/* |
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.
Is docprovider the right place for this, or should be in the collaboration
package?
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 asked myself the same question, and I put it here because it is really a document related widget, it supposed to handle the document current collaborators.
But I don't have a strong opinion on it, and can move it to collaboration
.
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.
Ok! I wouldn't know. Maybe @davidbrochart has an opinion on 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.
I would say that we should think about docprovider as enabling to use ydoc without collaboration
, so anything which is collaboration-specific should go to collaboration
unless it is impractical.
Should this be merged now while Or should this be part of a |
Right, we should probably wait for the next release. |
Looks like this repo does not make active use of GitHub milestones, but it could be useful to start using them to track such issues and improvements. I just created the following two milestones:
Feel free to move it back to the |
btw @brichet if you have a screenshot / screencast around that could be posted in the description that would be really useful, thanks! (and we could also add it to the changelog when releasing this) |
Thanks, that helped me to finish this PR, I had only tested it with jupyter-collaboration v2 and forgot to copy the styles in there. |
71edae1
to
bade577
Compare
70e5f92
to
c4d7d43
Compare
c4d7d43
to
aaaa156
Compare
aaaa156
to
14f42cb
Compare
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!
14f42cb
to
e1c28ad
Compare
Should we merge this PR, since we are currently in a RC release ? |
We can release v3.1.0 and then merge this PR, what do you think? |
Make sense, then we can probably release a 3.1.1, since this PR simply provide a new widget. |
If we're strictly following semver, that should be a 3.2.0 |
I don't mind having this PR in v3.2.0, but I think the release cycle is a bit overkill here (releasing alphas, betas, RCs...). |
You should be able to run prep-release with |
No, see the scheme to bump the version: jupyter-collaboration/scripts/bump_version.py Lines 15 to 52 in b3d8d29
|
Ok, it seems I'm used to releasing jupytercad which does what I said: https://github.com/jupytercad/JupyterCAD/blob/main/python/jupytercad/scripts/bump-version.py#L49 |
If there is a self-contained addition which warrants minor bump but not much else I am in favour of skipping some of the pre-release stages and e.g. going straight to RC (while bumping the minor) over just bumping the patch version. |
In the meantime, I think we can release v3.1.0, if everybody agrees. |
Sorry @brichet, but could you rebase one more time? |
…lay current users
…sor on Anonymous icons
- allow the default renderer to receive additional classes - rename it for consistency - avoid sending non related props to div element
e1c28ad
to
e588df7
Compare
Tests on Ubuntu all fail in |
I cannot reproduce locally on my Debian. |
Me neither on Ubuntu |
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.
Let's merge and address the CI issue on Ubuntu separately.
Thanks @brichet.
FYI @davidbrochart next time when making a final release, you can check the following box when triggering the workflow Otherwise the release does not show any merged PR, which is misleading: https://github.com/jupyterlab/jupyter-collaboration/releases/tag/v3.1.0 |
Right, I forgot about that. I'm wondering if it should not be checked by default. |
Copy of #378, to try to fix the
check_release
workflow.This PR provides a widget that can be used in a collaborative document to display current users.
This widget is a refactoring of a widget initially created in jupyterCAD.
Porting it to
jupyter_collaboration
has been initially discussed in jupytercad/JupyterCAD#496.EDIT:
If the widget is integrated to a notebook for example, it looks like:
Code for the notebook widget (in Jupyterlab)
The Notebook is a special case because the document model can be null when first displayed. We need to handle the
modelChange
event.packages/notebook-extension/src/index.ts
packages/notebook-extension/schema/panel.json