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.

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

Collab Highlight #141

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

arshiannafi
Copy link

Cherry picked commits

rsenekal and others added 8 commits August 10, 2017 13:17
- Setup Sidney's commit for collaboration
- Base collaboration works without highlight
Added collab highlight implementation to Sidney's clean collaboration pr/46
- Move div creation in a class function
- Eliminate redundant:
  - Creation of highlight divs
  - Reassignment of constants variables
Reflects the current data structure
Removes highlight when peers leave.
E.g. exit browser without deselecting the text.
Collab Highlight was originally built on an older commit. When collab highlight
code was copied to commit cb48ab, some code were altered as a side-effect.

In this commit, commit cb48ab (base) and 5e6c9d1 (head) was diff-ed
to find and restore altered code.

Restoring code from

1.
commit cb48ab8
Author: libing wang <[email protected]>
Date:   Thu Jun 22 15:04:33 2017 -0400
Subj:   Bug 514792 - AnnotationStyler to merge html and DOM node style in com...

Side-effect:    Around line `create: function(_parent, div, drawing) {`,
                all instance of 'drawing' was removed
In this commit: restored 'drawing'

2.
commit c81fb19
Author: Sidney <[email protected]>
Date:   Thu Jun 15 16:53:04 2017 -0400
Subj:   cleaned 50% garbage code

Side-effect:    '/file/' came back.
In this commit: removed '/file/'
Collab highlight width was inconsistent across different browsers.
The width would vary on different browsers as peers select long texts
on the same line. This is due to the use of a calculated constant in the code
rather than using environment variables that we were unaware of at the time.
Thanks to Silenio, we found a variable to fixes this issue.
@arshiannafi arshiannafi mentioned this pull request Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants