-
Notifications
You must be signed in to change notification settings - Fork 45
Feature/pub 1765 annotations examples #2636
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
base: main
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Uses a double-pane view to show the example side by side for two different clients.
Shown in an expandable pane when the message is clicked.
Publish an annotation, specifying the annotation type. Summaries are received as a message in the subscribe listener.
Listens for annotation summaries received in realtime and renders their information for each summary type in a set of expandable summary components in the message details.
Separates the annotations summary from the raw annotations messages displayed in separate tabs.
Adds a delete button to raw annotations to publish a delete annotation which removes the annotation from the summary. Refactors the raw annotation view to display in a more compact format.
Various improvements to the components and styling to create a more compact visual style.
Adds the franken-ui dependency to the examples renderer. Updates the set of visible files configured for sandpack.
4ea10b5
to
43e3aa9
Compare
7f3370d
to
f7a321a
Compare
|
||
## Technical notes | ||
|
||
- Annotations require a channel in the mutable channel namespace. This example uses `mutable:pub-sub-message-annotations` |
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.
- Annotations require a channel in the mutable channel namespace. This example uses `mutable:pub-sub-message-annotations` | |
- Annotations require a channel in the mutable channel namespace. This example uses `annotation:pub-sub-message-annotations` |
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.
Now I've reread the sentence, should it be the annotations channel namespace
or is mutable
the correct term?
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 re-worded to make sense now, maybe something like:
"Annotations need to be enabled for a channel, or namespace using a rule." This example is using the annotation
namespace but it isn't relevant to someone else implementing them, as I doubt they'd use that namespace.
yarn run pub-sub-message-annotations-javascript | ||
``` | ||
|
||
7. Try it out by opening two tabs to [http://localhost:5173/](http://localhost:5173/) with your browser to see the result. |
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.
7. Try it out by opening two tabs to [http://localhost:5173/](http://localhost:5173/) with your browser to see the result. | |
7. Try it out by opening two tabs to [http://localhost:5173/](http://localhost:5173/) with your browser to see the result. Specify different client IDs in the URL (e.g., `?clientId=user1` and `?clientId=user2`) to see how annotations from different clients are handled and summarized |
## Technical notes | ||
|
||
- Annotations require a channel in the mutable channel namespace. This example uses `mutable:pub-sub-message-annotations` | ||
- Annotations are logically grouped by an annotation namespace. This example uses `my-annotations` |
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.
@mschristensen please can you sanity check all the naming lines up? I'm not convinced this is the correct namespace because I can't see it specified anywhere in the code (other than a comment)
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.
(For reference, I finally managed to connect the dev console to the default channel and it's called annotation:annotation:pub-sub-message-annotations
)
Description
Adds an example app for the new message annotations feature. Showcases:
I recorded a quick Loom to walk through this example: https://www.loom.com/share/e8e37e37d7c04999b82493142b445336?sid=25392ef5-5d8e-4225-8bea-1e2f08fb31f4
I intend to update the docs before landing this PR, but opening now for draft review.
This examples requires us to configure a namespace (channel rule) with the
mutableMessages
flag enabled. We need to identify how we can do this when used from the examples.This also needs an image to include on the examples index.
Checklist