WIP: Optimize RTC Updates and RTC Highlighting #371
Conversation
|
Please resolve the conflicts and ping here :) I'll review it thereafter. |
|
@kdexd Conflicts resolved |
caffe_app/consumers.py
Outdated
| user_id = data['userId'] | ||
| highlight_color = data['highlightColor'] | ||
| username = data['username'] | ||
| print(data['username']) |
| @@ -8,7 +8,7 @@ def check_login(request): | |||
| try: | |||
| user = User.objects.get(username=request.user.username) | |||
| user_id = user.id | |||
| username = '' | |||
| username = 'Kafka' | |||
There was a problem hiding this comment.
Do we need to keep the default user name as this? Because in the open link sharing case, all users will be given the same user name and it might be confusing. For now can we do something simple like User1, User2 etc. ?
There was a problem hiding this comment.
We can do select from a pool of usernames, but then we'll have to figure out some method for each user having a unique name from the pool. Because there's no place where we can manage details of a session other than websocket pipeline
| this.changeCommentOnLayer = this.changeCommentOnLayer.bind(this); | ||
| this.getRandomColor = this.getRandomColor.bind(this); | ||
| } | ||
| getRandomColor() { |
There was a problem hiding this comment.
Instead of random colours, can we choose from a colour palette to ensure the colours are different? Because in some cases they are very similar. We can have a set of say 10 colours and then reuse them if there are more users.
There was a problem hiding this comment.
We may get repitition here, I wasn't able to figure out a way to completely ensure colors are not repeated for multiple users. Similar to username problem
ide/static/js/content.js
Outdated
| @@ -129,19 +143,60 @@ class Content extends React.Component { | |||
| onSocketMessage(message) { | |||
| // message received on socket | |||
| let data = JSON.parse(message['data']); | |||
| let rebuildNet = false; | |||
| let nextLayerId = this.state.nextLayerId; | |||
| //let rebuildNet = false; | |||
ide/static/js/layer.js
Outdated
| highlightColor = this.props.layer.highlightColor[this.props.layer.highlightColor.length - 1]; | ||
| highlightUser = (<div style={{ | ||
| background: highlightColor, | ||
| color: 'white', |
There was a problem hiding this comment.
Can we shift these style specifications to the css stylesheet as they are fixed?
There was a problem hiding this comment.
Sure, but highlight color is dynamic
* RTC comment UI improvements * Improvising UI * Optimze RTC update and add highlight initial work * Add complete support for RTC checkpointing * Add support for RTC checkpointing comments * Complete model history with checkpointing * Add support for RTC highlighting * Fix issues in username in highlight * Change comment ref name * Remove debugging statements and minor fixes
Work in progress