Conversation
…t an id that can be joined using /collab/create?id endpoint
timothycpoon
left a comment
timothycpoon
left a comment
There was a problem hiding this comment.
Hey Jonathan, all of this looks good. Just a couple things to fix, and then we need to wait on a collab-join link to merge. Thanks!
| window.addEventListener("close", this.onLeave); | ||
| } | ||
| try { | ||
| let collabId = await sketch.constructCollabId(this.props.uid); |
There was a problem hiding this comment.
If we create a collab session every time someone opens the editor, we will end up creating a lot of sessions that will never be used. I think it would be best to make creating a session an explicit action.
There was a problem hiding this comment.
should I create a separate collab modal to create a collab session or should I put a create collab button under the Share modal?
There was a problem hiding this comment.
I was thinking we could have a "Create Collab Session" button, which would then generate and display the URL. If you feel strongly either way you can do that though.
src/lib/fetch.js
Outdated
| * @param {string} id //id of CollabSession | ||
| */ | ||
|
|
||
| export const joinCollab = async (id) => { |
There was a problem hiding this comment.
I believe that joining a session will be handled by the "collaborative mode" of the editor, so we probably don't need this function. However, if we do end up using this function, there are a couple issues: first, the currently planned endpoint would be collab/join/:id. Secondly, it should open a websocket, rather than make a GET request.
| initiateCopy = () => { | ||
| navigator.clipboard.writeText(this.props.shareUrl).then( | ||
| initiateCopy = (textToBeCopied) => { | ||
| console.log("textToBeCopied is: ", textToBeCopied); |
There was a problem hiding this comment.
Should probably remove debugging console.log
|
@jonathanbcarlson Hey, could you gate this feature behind |
2 participants
waiting to merge until able to join collab session with url