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

Annotations on dynamic content and controlling what can/can't be annotated #6

Open
cdunlap opened this issue Feb 10, 2015 · 6 comments

Comments

@cdunlap
Copy link

cdunlap commented Feb 10, 2015

Hello! Factlink will most likely save me a ton of time writing my own annotation system, so what you guys have created is much appreciated. So before I start in with the issues, I'd like to say thank you :)

First, I'd like to be able to annotate content that is loaded via AJAX. Right now that isn't possible. Is there some method I can call to have it re-parse the DOM and populate the annotation buttons again after content is loaded? This seems like a pretty simple thing to do, but I could totally be wrong. I looked through the code on GitHub and couldn't really find a clue to help with this.

Second, is there a way to control what is allowed be annotated and what isn't? IE: Article content paragraphs only, using some sort of class added to the elements that can't be annotated. Or perhaps by passing a selector to Factlink to tell it what IS annotatable.

Thanks in advance!

@janpaul123
Copy link
Member

Hey, thanks for using our software. Always great to see!

Both things are currently not officially supported, although we did think about them.

  1. I hope to be able to make a pull request soon that adds official support for this, but for now you can use factlink_jail_iframe.contentWindow.FactlinkJailRoot.highlightAdditionalFactlinks(window.location). This is very hacky though. You will probably also get some errors in the console if you already have some annotations loaded already. Note that we also don't support overlapping annotations, unfortunately. If you're interested in the code that you'll call, here it is:
    FactlinkJailRoot.highlightAdditionalFactlinks = (siteUrl) ->
  2. You cannot exclude parts of the page from Factlink as far as I know. But you can specify where the paragraph icons should appear. If you create containing <article> element (or <div class="article">) around your content, then within there paragraph icons will appear for each <p> or <div class="paragraph"> and so on. See https://github.com/Factlink/js-library/blob/23264ca16bae7982c68dcc411aa15e86686e76ef/app/js/jail_iframe/classes/paragraph_buttons.coffee#L40-42 and
    ['p', 'li', 'dd', 'dt', '.paragraph', '.para', '.par', '.text', '.summary']

Hope this helps at least a bit. cc @markijbema @EamonNerbonne @martijnrusschen

@cdunlap
Copy link
Author

cdunlap commented Feb 11, 2015

Yeah this helps a lot! Thank you! I'll be giving it a shot tomorrow when I get into the office. I'll let you know the result.

Thanks again!

@EamonNerbonne
Copy link
Contributor

If you're bugged by the console errors, these can be disabled in highlight.coffee:111 by making the else a little more specific (or simply removing the warning altogether). Perhaps the best solution would be to allow checking for the existance of a particular factId. The data structure to do that already exists; it's just not part of the public api: that's highlightsByFactIds, which stores highlights in a hash by their id.

@cdunlap
Copy link
Author

cdunlap commented Feb 11, 2015

@janpaul123 Calling factlink_jail_iframe.contentWindow.FactlinkJailRoot.highlightAdditionalFactlinks(window.location) after the new content is loaded in via AJAX didn't seem to have an effect on anything. The content that was loaded via AJAX still doesn't allow annotations (no annotation buttons). Only the content that was present on DOM ready is annotatable. The content IS however annotatable when the user selects a block of text, rather than hovering over a paragraph.

Any other ideas?

@cdunlap
Copy link
Author

cdunlap commented Feb 11, 2015

I'm no CoffeeScript developer, but I took a crack at hacking together a function to call. I noticed that in paragraph_buttons.coffee there is a callback attached to the host_ready_promise that instantiates the ParagraphButtons class and calls addParagraphButtons()

I basically turned that into a function hanging off of FactlinkJailRoot:

FactlinkJailRoot.refresh_paragraph_buttons = () ->
  console.info 'Refreshing paragraph buttons'
  paragraphButtons = new ParagraphButtons
  paragraphButtons.addParagraphButtons()

Do you guys see a problem with this at all? It doesn't seem to be causing any errors or anything on the site.

@EamonNerbonne
Copy link
Contributor

Unfortunately, we've got lots of side-effectful constructors, so it's a little hard to follow, but I think there is a problem with that; namely that addParagraphButtons adds buttons regardless of whether they've been added before, i.e. that adds duplicate buttons to paragraphs already. I haven't tried this, but that strikes me as not immediately problematic, but certainly not the way we'd want to keep things.

In other words, you'd need to alter addParagraphButtons so that it doesn't add buttons to paragraphs that already have a button. That should be good enough, even though to be 100% correct you'd really want to avoid adding buttons such that the paragraph is contained by or contains a different "paragraph" that already has a button. If you wanted to solve that, you could remove all paragraph buttons and recreate them all; a paragraph button has no interesting state that can be lost.. That's likely going to be a little slow; an alternate solution would be to recompute the paragraphs needing buttons from scratch, adding new buttons where necessary, leaving buttons that already exist correctly in place, and removing buttons.

So either:

  • remove old buttons before adding them again
  • accept duplicate buttons (really hackish, this needs to have a big warning if the function is public)
  • add buttons only the paragraphs that don't have them (not 100% perfect, but good enough)
  • recompute the new set of paragraphs and add/remove/keep icons based on how the new set differs from the old.

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

3 participants