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.

Sign up for GitHub

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

Jump to bottom

Chatbox #2118

Merged
merged 42 commits into from
May 25, 2017
Merged

Chatbox #2118

merged 42 commits into from
May 25, 2017

Conversation

ian-r-rose
Copy link
Member

This is a start of a chatbox widget for collaborative documents. It lives on the sidepanel and shares the same collaborator pool as the currently active collaborative document. It is not fully functional at the moment (contingent on #2058), but has the right basic behavior.
chat

Things that currently work:

  • Multiple users, with different color text bubbles.
  • Rendering with markdown.
  • Showing different chats depending upon the active document.

Things that are not currently working:

  • Some variety of paging for long chatlogs.
  • It would be cool to be able to drag chat entries into a notebook as markdown cells.

cc @ellisonbg @cameronoelsen

@ellisonbg
Copy link
Contributor

ellisonbg commented Apr 27, 2017 via email

@jasongrout
Copy link
Contributor

Very cool. It would be great to support mathjax too.

@ian-r-rose
Copy link
Member Author

MathJax works. The chat entries are just markdown cells with some stripped-down styling.

@jasongrout
Copy link
Contributor

CC @williamstein, who has something like this in SMC. William, do you have any suggestions or tips from your experience with chats associated with documents?

@sccolbert
Copy link
Contributor

That. Is. Slick.

@jasongrout
Copy link
Contributor

I can imagine this being particularly nice in single-document mode - change the document, and the chat channel changes to the new document.

@cameronoelsen
Copy link
Contributor

This is sweet @ian-r-rose! Pinging @faricacarroll, we had a meeting yesterday conceptualizing the initial chat experience and she should have some wireframes/mockups in the next week!

afshin
afshin reviewed Apr 27, 2017
View reviewed changes
packages/chatbox/src/chatbox.ts Outdated
}
set model(model: DocumentRegistry.IModel) {
this._model = model;
let modelDB: any = (this._model as any).modelDB;
Copy link
Member

Choose a reason for hiding this comment

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

Is this casting as any temporary until the real-time handler PR goes in?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, there are a few temporary stubs in this PR that I will take care of after the #2058 is resolved.

@ian-r-rose ian-r-rose force-pushed the chatbox branch 2 times, most recently from 494d542 to a859e85 Compare May 3, 2017 15:19
@williamstein
Copy link

@jasongrout since you asked, here is the analogue in SageMathCloud of the above screenshot:

screen shot 2017-05-04 at 5 55 28 pm

@ian-r-rose
Copy link
Member Author

@williamstein thanks for the input.

@Analect
Copy link

Analect commented May 10, 2017

@ian-r-rose
Could I ask where the chat thread gets persisted? Does it form part of the meta-data in an *.ipynb file?

If it's not persisted in the notebook, was this your motivation for allowing for parts of the thread to get dragged to a markdown cell in the notebook (I think you are doing this in widget.ts?) as an alternative form of persistence?

@ian-r-rose
Copy link
Member Author

@Analect We have been working on an abstracted database for storing document models that different backends can implement. The chat log gets stored in that database, and is persistent if the database is. The default, in memory notebook is not persisted except through the normal save mechanism, so the chatlog would likewise not be persisted for that case (in fact, I am not even showing it in that case). It is not currently serialized as part of the *.ipynb file, though we may have some discussions along those lines.

The markdown cell dragging is just something that we thought would be neat, and perhaps useful to some people. Since I was already representing chat entries internally as markdown cells, it was relatively easy to add.

@Analect
Copy link

Analect commented May 10, 2017

Thanks @ian-r-rose for the quick response. So just so I'm clear, is your work on widget.ts about dragging markdown cells from your notebook to a chat thread ... or the other way around? Just trying to get a better understanding (on jupyterlab gitter here) of emulating your approach for something not necessarily involving a chat thread ... but where I want to inject pre-existing *.md files in order to give a skeleton to an *.ipynb file ... and ideally do this programmatically.

@ian-r-rose
Copy link
Member Author

At the moment I only have dragging of chat cells into a notebook, not the other way around. It would not be too much work to do the other way, though.

@ian-r-rose ian-r-rose changed the title [WIP] Chatbox Chatbox May 24, 2017
@ian-r-rose
Copy link
Member Author

There is more that can be done with the styling and UI, but I am fairly happy with the basic functionality here. Note that the chatbox tab does not even appear unless a collaborative document is made active.

@blink1073
Copy link
Contributor

Nice! That was one concern I was going to bring up.

@ellisonbg
Copy link
Contributor

I am fine with us merging this and having @cameronoelsen do a design pass. Ready to go?

@ian-r-rose
Copy link
Member Author

I am happy with it. I expect a couple of minor conflicts with #2232.

@blink1073
Copy link
Contributor

I'd like to do another review pass.

blink1073
blink1073 reviewed May 25, 2017
View reviewed changes
packages/default-theme/style/icons.css Outdated
background-image: url(icons/md/chat.svg);
}

.jp-CloseIcon {
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant definition

@blink1073
Copy link
Contributor

Nice!

@blink1073 blink1073 merged commit 8dc8360 into jupyterlab:master May 25, 2017
@ian-r-rose ian-r-rose deleted the chatbox branch May 25, 2017 19:46
@blink1073 blink1073 mentioned this pull request May 30, 2017
Closed
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@blink1073 blink1073 blink1073 left review comments

@afshin afshin afshin left review comments

@ellisonbg ellisonbg ellisonbg approved these changes

Assignees
No one assigned
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. status:Work in Progress tag:Real Time Collaboration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ian-r-rose @ellisonbg @jasongrout @sccolbert @cameronoelsen @williamstein @Analect @blink1073 @afshin