TEIC / CETEIcean

TEI in HTML5 Custom Elements
BSD 2-Clause "Simplified" License
164 stars 36 forks source link

separated the domToHTML and register elements functionality #16

Closed jeffreycwitt closed 6 years ago

jeffreycwitt commented 7 years ago

I separated the domToHTML and register elements functionality so that…they could be used independently.

I needed to do this because I wanted to provide a live tei preview rendering as the user types. When the two functionalities are coupled, an error occurs because it says the elements have already be registered.

Separating things this way let's be re-rendering the tei to html elements without re-registering the elements.

Let me know what you think.

jeffreycwitt commented 7 years ago

Here's example of the case where I needed this functionality:

https://youtu.be/syfuhffNAYI

raffazizzi commented 7 years ago

@jeffreycwitt amazing stuff! Wonderful to see CETEIcean used in this way, as this is precisely the kind of software I was hoping it would allow to create. I'll do a few checks to make sure we can include this code, which would be a useful improvement. Any other thoughts @hcayless?

hcayless commented 7 years ago

A couple of questions: is an error actually thrown, or just logged? Element registration is wrapped in a try/catch, so it oughtn't to be breaking stuff, I'd have thought (though it might get unpleasantly shouty). Also, does this risk creating elements that aren't registered? We might be better off keeping track of what is and isn't registered in that case, and make sure we only register new stuff.

hcayless commented 7 years ago

And let me just echo @raffazizzi's comment: this is pretty awesome!

raffazizzi commented 7 years ago

@hcayless I think it's thrown but it's not fatal. I've run across it before when "paginating" between different TEI files with a <surface> each.

jeffreycwitt commented 7 years ago

Here's the basic error:

tei-teiHeader couldn't be registered or is already registered. CETEI.js:1 DOMException: Failed to execute 'registerElement' on 'Document': Registration failed for type 'tei-teiheader'. A type with that name is already registered. at r.value (http://localhost:4567/CETEI.js:1:6802) at r.value (http://localhost:4567/CETEI.js:1:4231) at HTMLTextAreaElement. (http://localhost:4567/edit?resourceid=ahsh-l1p1i1t1q1c1&reponame=summahalensis&branch=student-work:419:13) at HTMLTextAreaElement.dispatch (http://localhost:4567/mirador-2.4/mirador.js:5:10315) at HTMLTextAreaElement.q.handle (http://localhost:4567/mirador-2.4/mirador.js:5:8342) CETEI.js:1 tei-fileDesc couldn't be registered or is already registered.

This error does seem to prevent the tei preview from being updated.

hcayless commented 7 years ago

Hmm. It shouldn't, because try/catch is supposed to prevent it. But the browser DOM is a twitchy thing. And by twitchy, I mean horrible.

Here's the scenario I worry about: you're register elements separately from converting them, so you can just register them all at once, but convert as many times as you like. A user of your application enters a new, hitherto unseen, but legal element. How does it get registered? What I'm wondering is whether it might be better for us to keep a running list of registered element names and check against it before registering new ones, which would avoid triggering the error.

jeffreycwitt commented 7 years ago

That makes sense.

1) So starting with an empty array of registered elements, the first registration would compile all new elements and add these elements to the previously empty array. All further conversions would check the array, before registering a new element.

If there are a lot of elements, do you think this look up could get computationally expensive.

2) Alternatively, could we allow the implementer a choice of pre-loading all "legal" elements in their schema? In this case, when the user loads the page, all valid elements get pre-registered. Then nothing new would need to be registered.

Option 1 is probably simpler, assuming we don't foresee the "look-up" time as computation expensive.

jw

On Thu, May 11, 2017 at 4:52 PM, Hugh A. Cayless notifications@github.com wrote:

Hmm. It shouldn't, because try/catch is supposed to prevent it. But the browser DOM is a twitchy thing. And by twitchy, I mean horrible.

Here's the scenario I worry about: you're register elements separately from converting them, so you can just register them all at once, but convert as many times as you like. A user of your application enters a new, hitherto unseen, but legal element. How does it get registered? What I'm wondering is whether it might be better for us to keep a running list of registered element names and check against it before registering new ones, which would avoid triggering the error.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TEIC/CETEIcean/pull/16#issuecomment-300913470, or mute the thread https://github.com/notifications/unsubscribe-auth/ABF_PXSLaMMdQ840dImnzpmZslQKdLEAks5r43T6gaJpZM4NYJpt .

-- Dr. Jeffrey C. Witt Philosophy Department Loyola University Maryland 4501 N. Charles St. Baltimore, MD 21210 www.jeffreycwitt.com

raffazizzi commented 7 years ago

@jeffreycwitt I think for the time being the best option is 1). Would you want to implement and update your PR? Re: option 2, I think that's something that we may want at some point: either in the form of a list that the user provides (but that they would have to somehow generate) or, better, from a TEI ODD in JSON form (which can be obtained from http://www.tei-c.org/oxgarage/ and, hopefully soon, Roma).

hcayless commented 7 years ago

If we use an object to store the registered element names, then lookups should be O(1) and the performance impact tiny. Such a property could be set either automatically, in the process of loading a TEI document, or manually (if you had an ODD). Something like:

this.regelts = {};

at initialization, and then

if (!regelts[prefixedName]) {
  document.registerElement(prefixedName, {prototype: proto});
  regelts[prefixedName] = true;
}

should do it. I'm wondering whether this migh make for a smarter replacement for this.els...have to think about it.

jeffreycwitt commented 7 years ago

Just a follow up, given more progress using this modified version of CETEIcean.

In this demo https://youtu.be/CJ4oAp176dE, I don't see the problem arising as described by @hcayless. The concern was:

Here's the scenario I worry about: you're register elements separately from converting them, so you can just register them all at once, but convert as many times as you like. A user of your application enters a new, hitherto unseen, but legal element. How does it get registered? What I'm wondering is whether it might be better for us to keep a running list of registered element names and check against it before registering new ones, which would avoid triggering the error.

But as you can see in in the demo, in the editor I've added the elements <choice> <reg> and <orig> and CETEIcean still seems to be able register these new elements and style them appropriately.

hcayless commented 6 years ago

This isn't mergeable, given the changes to CETEIcean, but the applyBehaviors() method should supply this functionality.