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

Re-design highlighting logic to not affect caret position #31

Closed
sbrudz opened this issue Jan 27, 2016 · 0 comments
Closed

Re-design highlighting logic to not affect caret position #31

sbrudz opened this issue Jan 27, 2016 · 0 comments

Comments

@sbrudz
Copy link
Contributor

sbrudz commented Jan 27, 2016

Many of the outstanding bugs (#10, #12, #25, and #27) and the fixed bugs (#6, #8, #22) are related to how highlighting and cursor positioning are done. I've tried different fixes for the outstanding bugs but nothing has worked reliably and the code gets ugly quickly. When that happens consistently, it's a design smell and the design needs to be reconsidered.

The core issue is that when just-not-sorry highlights a word, it mutates the DOM. When words are edited (e.g., when "just for" gets changed to "justification for"), it needs to strip out the highlighting by deleting the DOM node. Deleting the node causes the cursor position to be lost. To address this, we programmatically set the cursor position back to what it was before, but this fails for certain edge cases (#10, #25) or has other side effects (#27). Finally, before the email is sent, it needs to strip out the highlighting, but for certain edge cases the stripping fails (#12).

The new design should do the highlighting without mutating the part of the DOM that contains the text you're editing.

An additional benefit of the redesign will be that we can more easily extend just-not-sorry to other web email clients (inbox, hotmail, etc. -- #4, #20) because we won't need to write and test new logic to strip out the highlighting for each email client.

The spike in commit c36981f shows that creating an overlay with the highlighting using Range.getBoundingClientRect is a viable solution.

This ticket is to track the work required to turn the spike into a production-ready implementation.

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

No branches or pull requests

1 participant