highlight is wrong when editing #182

Closed
tilgovi opened this Issue Oct 16, 2012 · 8 comments

2 participants

@tilgovi

No description provided.

@gergely-ujvari

I've made a change which corrects highlight when creating a new annotation (no-reply). Is it the problem you were thinking about?
It can be seen here: https://github.com/gergely-ujvari/h/tree/182-Highlight-is-wrong-when-editing

@tilgovi

Nice. Looks like the right bug. I'm not sure this is the best way to solve it, though.

Maybe break the highlighting code out of show() rather than introduce a new argument. Then we can be explicit and call it when we need it. show feels like it has overloaded duties right now.

What do you think?

@gergely-ujvari
@gergely-ujvari
@tilgovi

Just realized this:

In js/inject/host.coffee (what used to be annotator.host.coffee), setActiveHighlights() could be easily modified like so:

            if $(this).data('annotation').hash in hashes
              $(this).addClass('annotator-hl-active')
-            else
+            else if not $(this).hasClass('annotator-hl-temporary')
              $(this).removeClass('annotator-hl-active')
@tilgovi

That will keep the active edit from being un-highlighted. Then factor the highlight calls out of Hypothesis.show(). This way, you just need to prevent the other highlights from being active while editing, but don't have to worry about restoring the edit highlight or tracking it when the user hovers on the heatmap, etc.

@gergely-ujvari gergely-ujvari pushed a commit to gergely-ujvari/h that referenced this issue Nov 30, 2012
Ujvari Gergely Enhanced solution for issue #182 8eb1468
@gergely-ujvari

Yes. That works fine. I updated my branch. It has only one tiny problem. It you close the toolbar and open again it forgets the current highlight. Maybe we can do a restore when opening the toolbar. I've guessed to do the highlighting in the _setupDocumentEvents function but it did not produce a good output. Do you have any idea where to call the highlight when opening the toolbar? (That's why I left the factored out highlight function in the code)
(Or is it a too little matter, because when the user goes to the toolbar a new highlight will be selected anyway.)

@tilgovi

Closed since a923457 (angular-refactor merge)

@tilgovi tilgovi closed this Dec 19, 2012
@tilgovi tilgovi added a commit that referenced this issue Dec 19, 2012
@tilgovi tilgovi Merge branch 'angular-refactor' into develop
closes #219
closes #182
closes #162
closes #150
closes #104
a923457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment