search

Juris-M / citeproc-js-docs

Documentation for citeproc-js
13 stars 14 forks source link

Revert back to ES6 + add babel #4

Closed dsifford closed 5 years ago

dsifford commented 8 years ago

Thought I'd start a working PR here.

I'll add comments inline where I have questions.

Note: I'm not familiar with makefiles, but babel has an integration for it. For now, in order to compile to ES5, you just need to do the following.

  1. Pull the project
  2. npm install
  3. npm run build
dsifford commented 8 years ago

Also, if you look at the changes I made, there are several instances where you use the instance name within the class definition (ie citesupport). I went ahead and changed all of them to this.

dsifford commented 8 years ago

More notes:

Looks pretty good so far, but in order to make it consumable a few things need to be removed from the CiteSupport class....

  1. Any calls to a worker.
  2. Any DOM methods that are only necessary for the demo, or that aren't relevant anywhere but the demo (there's quite a few)

Basically if there is any use of document.getElementByX then it needs to either be refactored, or go completely.

I'm working on it on my fork, but I thought I'd just relay the message in the event you'd like to on your end as well.

Calling it a night there

dsifford commented 8 years ago

@fbennett Re: calling it a night: I lied.

Made some changes again.

Still lots more to do. Let me know what you think..

(To run this, run npm run reset or npm run build)

reset starts the server. build just builds the files. (Disclosure: The build process is very sloppy, but that can be fixed later)

fbennett commented 8 years ago

In this bit:

In _static/js/citesupport.es6:

> @@ -802,53 +838,61 @@ class CiteSupport {
>          this.safeStorage.citationIdToPos;

What is this? Should this be deleted?

That calls the safeStorage getter, initializing the variable in this.config. It doesn't serve any purpose outside of the demo.

fbennett commented 8 years ago

The calls to citesupport are in event context, where this is the bare event function (I think - in any case, it doesn't carry the methods of the class instance as far as I can tell, and in that context I normally wouldn't expect it to).

fbennett commented 8 years ago

Okay! Have cloned and configured and built. With the import in demo.es6 changed to "citeproc.es6," it spins up without complaining. I get a syntax error on the export declaration at the bottom of the bundle.js file, so the page doesn't populate; but other than that all seems green.

dsifford commented 8 years ago

That calls the safeStorage getter, initializing the variable in this.config. It doesn't serve any purpose outside of the demo.

Ah, I see... I removed that from this branch. So that might be why you're still getting barked at.

The calls to citesupport are in event context, where this is the bare event function

Gotcha. Yeah, you can get around that by binding the current state of this to the event handler. See here

I get a syntax error on the export declaration at the bottom of the bundle.js file, so the page doesn't populate; but other than that all seems green.

Not getting that on my end, but not super worried for now. Still totally a WIP.

I'll keep you posted as things come along. I'll dig back into this later on today.

fbennett commented 8 years ago

Gotcha. Yeah, you can get around that by binding the current state of this to the event handler. See here

Ah, much better. Lack of study - I had assumed that was a Firefox-ism (most of the JS I've written was written to run in Zotero - the exception to that is the processor, but that's all blocking functions internally).

dsifford commented 8 years ago

@fbennett I added a better config file for webpack. You shouldn't be getting errors any longer if you pull it in and run again.

(you'll have to run npm install beforehand because I added another dev dependency)

dsifford commented 8 years ago

The biggest challenge I'm having is removing the webworker (since I really have no idea how they work).

It'll need to be gone and everything done in-house in order to create a consumable package

dsifford commented 8 years ago

@fbennett I'm converting my fork to TypeScript because I'm finding it too difficult to try and remember what's what.

Once I get a handle on it, I can convert back if that's what you prefer

dsifford commented 8 years ago

Just some notes...

These methods within the CiteSupport class need to be removed (they're DOM methods. This class needs to ONLY be concerned with obtaining and serving data, otherwise this will not be reusable anywhere but in this demo).

dsifford commented 8 years ago

I'm noticing now that this is 8 of 14 functions within the class. Not sure if this will be usable to be honest. I'll try and see if I can understand the worker and then I'll probably have to rewrite it so it consists of only pure functions that get and serve data (no DOM methods).

Edit: See my borderline-insane proposition in your main repository.

dsifford commented 8 years ago

Update: I think I figured out a way to be able for my Wordpress app to handle the stateful data necessary (but it's going to take a huuuuuge refactor).

fbennett commented 8 years ago

I don't see the proposal you mentioned. I don't really see how you can remove DOM access everywhere. The citation data registered in the processor needs to be aligned with document state, which can change between processor transactions through addition, rearrangement, pasting, and removal of citations. The way that's done in a word processor is through a platform-specific API to the document that returns the essentials of the current state. That's mostly what this is - as you say, there are very few functions in there that deal with data alone, and that's why. So I'm kind of at a loss at what you have in mind, but I'm happy to watch and learn.

dsifford commented 8 years ago

Sorry for the delay -- Making giant strides on my WordPress project...

I don't see the proposal you mentioned.

I opened a new issue in your main citeproc repo. The idea of rewriting the code in TypeScript came to mind (see issue for further details --- Yes, I think I might be crazy, but it seems like it might make for a cool project and a fun learning experience)

Re: Removing DOM from the processor. IMHO, the processor should only be concerned with managing the global processor state and the application should only be concerned with presenting that state in whatever way that makes sense for that application.

In my mind (which is likely a drastic oversimplification, so forgive me), each action on the processor should be explicit (eg, add a citation, remove a citation, etc). You should be able to send the processor some representation of the action that you're looking to perform on the processor and receive a payload containing an Object representing the entire state after the action has been performed. From there, the application can manage the data presentation (without having to manage its state (or even being aware of it), and the processor can manage the state.

(One area that still needs some thought here would be how to restart the processor again, say, if the user saves and then returns at another time).

One other feature that would totally be awesome is if there was the ability to specify what pieces of information you require for your specific application (and even define its shape). That way, the processor always keeps a full representation of every piece of information it could ever need, but the application only receives what's relevant to them, in the shape that they define, after an action is invoked on the processor.

Just thinking out loud again. Curious to hear your thoughts.

dsifford commented 8 years ago

I suppose another thing I'm leaving out of my equation is the fact that there's really two unique "states" -- The state of the bibliography, and the state of citations (which, at least in my case, could live in two very different locations)....

Still lots to mull over (likely things you've already mulled over years ago) -- I apologize in advance if I'm sounding like a buffoon here...

Edit: The more I become familiar with the processor in its current form, the more I'm beginning to realize that it's actually performing much like how I described above... Feel free to stop me if that's the case.

(makeBibliography in this case performs pretty much exactly how I described... I think I should probably keep familiarizing myself with what you've put together before getting to far ahead of myself)

fbennett commented 8 years ago

Yes, I was going to pitch in a comment there. The processor knows the current document state -- not the rendered strings, but the knowledge needed to render them correctly. When a fresh body of state information is registered in the processor, the state information is updated across the board, and it spits out the strings that should be merged back into the document.

One thing to note about styles in the processor: you can't update styles on the fly in a running instance of the processor. You can change the output mode (from HTML to RTF or plain text, say), but you can't change the style -- to do that, you have to reinstantiate the processor from scratch, and then (to recover state) send an array of citation objects to rebuildProcessorState(). That's by design: styles are compiled, not interpreted, and the state data each carries is specific to the rules of the style.

If you want to see the range of things controlled by the processor, take a look at the CSL Specification.

fbennett commented 8 years ago

I've taken a look at the tinyMCE source, and it looks like we can adapt the contextmenu plugin to run the demo citation widget. If I set that up along with a method for embedding data in the citations, and for storing and recalling the serialized document text from localStorage, all that would remain would be to implement a proper cite-as-you-write selection plugin, and provide a proper save mechanism. In your WordPress environment, you could do up something that searches only the items held in the sidebar---if the JSON for those can be fetched via XMLHttpRequest(), it should run (it says here :).

Not 100% confident about the details, but it would be fun to add it to the demos, so I'll tinker away at it and see how it goes.

dsifford commented 8 years ago

Awesome. I dig the idea.

I've made some gigantic strides in my current implementation (If I had to gauge, I'd say I'm a third of the way done).

If you want to check it out, pull the "next" branch from my repo and check out the ReferenceList.tsx, API.ts, and CSLProcessor.ts files. I added an immutability helper library which has been making my life a bit easier (Immutable.js by Facebook)

One question that I have that just came up: How do you implement margin details for various citation styles? Put a different way: given the following....

<div class="csl-entry">
    <div class="csl-left-margin">2.</div><div class="csl-right-inline">Carver, F. &amp; Frieden, E. Factors affecting the adenosine triphosphate induced release of iron from transferrin. <i>Biochemistry</i> <b>17,</b> 167–72 (1978).</div>
  </div>

How are you able to determine how to set the css for the left-margin? I know that some citation styles differ with their list margins in the follwing ways:

Example 1

1. Lorem ipsum dolor sit amet, sociosqu laoreet turpis lorem penatibus pede viverra, 
   ugue turpis placerat hendrerit adipiscing consectetuer, repellendus magna ligula
   vestibulum fermentum non.

Example 2

1. Lorem ipsum dolor sit amet, sociosqu laoreet turpis lorem penatibus pede viverra,
augue turpis placerat hendrerit adipiscing consectetuer, repellendus magna ligula
vestibulum fermentum non.

...etc.

Ideas?

Also, do you have a recommendation on how to create a new instance of the citeproc object using existing data?

Re: How to persist data. I think I'm going to store the required data as serialized JSON in a hidden field and save it to the database (as post meta) when people save their posts. That way it'll always be available without any fear of someone deleting it by accident.

dsifford commented 8 years ago

Re: My margin question -- I think I might understand how it works.

It looks like options are set in the opt object for this. Would hard-coding css instead of doing that be more efficient? That way less object lookups will be necessary..

So for example....

If the hanging-indent (or maybe second-field-align? not sure) specifies that the citation style requires the field to be like example 1, give this:

<div class="csl-entry" style="display: flex;">
    <div class="csl-left-margin">2.</div><div class="csl-right-inline">Carver, F. &amp; Frieden, E. Factors affecting the adenosine triphosphate induced release of iron from transferrin. <i>Biochemistry</i> <b>17,</b> 167–72 (1978).</div>
  </div>

(Note: probably would have to use something more cross-platform reliable -- flexbox is still kinda shaky)

and if it specifies something like example 2:

<div class="csl-entry">
    <div class="csl-left-margin" style="float: left;">2.</div><div class="csl-right-inline">Carver, F. &amp; Frieden, E. Factors affecting the adenosine triphosphate induced release of iron from transferrin. <i>Biochemistry</i> <b>17,</b> 167–72 (1978).</div>
  </div>

Just an idea... Although perhaps I'm overlooking how it actually works (as usual). I'll wait to hear back. 👍

dsifford commented 8 years ago

Just noticed your documentation on callInitProcessor you are the man! Reading through it now...

fbennett commented 8 years ago

Bibliography formatting is hackish. The processor returns some configuration parameters in the first of the two array elements. The source code is the best reference, unfortunately, but it's here. The CSL spec explains what the linespacing, second-field-align and hangingindent options are meant to do.

The maxoffset value is meaningful when second-field-align is used. It's just a count of the characters in the "first field" (the label). Generating an offset from that value is pretty much guesswork, and fragile. It's not great, but it's about the best we could do, since the processor must work for both RTF and HTML, which have very different capabilities.

dsifford commented 8 years ago

@fbennett Thanks. I'll have to look it over some.

One question that just came up actually is related to this object (the first arg to processCitationCluster, which looks to have the same shape as a single citationByIndex):

image

I'm trying to determine what each piece is used for and where they come from. Here's my current understanding.

citationID: The HTML ID for the span tag of interest. citationItems: Array of objects (ordered by occurrence in the bibliography) with id mapped to the citationID stored in the processor. properties: I have no idea what this is, or what it's for. sortedItems: Array of tuples with the following shape:

type sortedItems = 
[ 
    Object, // CSL JSON Object
    {
        id: string // the citation ID stored in the processor -- same as in citationItems
        item: Object // CSL JSON Object (not sure why this is being reused here)
        'section_form_override': string // Not sure what this is for
    }
][]

Could you clarify?

dsifford commented 8 years ago

Also, citationsPre and citationsPost seems to be referring to how many inline citation span elements are before (pre) and after (post) the current HTMLSpanElement of interest.

Is this needed? I can't seem to deduce why the processor would need to be concerned about where the citation is in the document if all we need to do is update a single elements inline citation tag? (I'm probably missing an important point here)

fbennett commented 8 years ago

first arg to processCitationCluster

Yep, that's a pointer direct to an element in citationByIndex. It represents one citation. It contains a lot of stuff used by the processor internally, most of which you can ignore.

citationID

Yes, exactly. That's the ID assigned to the citation when its data is first registered in the processor. If the ID is written into the document citation node as its ID, it provides an addressing mechanism shared by the processor and the document.

citationItems

A list of citation items. For the document-to-processor submission, only the id and optional pinpoint information (label and locator) is needed, since the processor will refresh this data anyway. It items are in the sequence set by the user. Some styles will sort citations by author or date or whatnot, and the sort-ordered items appear in sortedItems.

sortedItems

You can ignore this, it's used only internally by the processor, and shouldn't be touched.

properties

This carries noteIndex, which is the note number of the citation. For in-text (author-date) styles, references in the text itself have noteIndex of 0. In the demo, it can be derived from the citation index (as pos + 1, in note styles only), but in word processors that is not always true---users can manually create footnotes, so the number of individual citations needs to be reported to the processor explicitly.

citationsPre, citationsPost

This is definitely needed. Citations are context-sensitive, so the sequence and grouping of items is important (for example, "Ibid" cites are used when there is exactly one reference in the immediately preceding reference, and it is the same as the first reference in the current citation). If a citationID is not included in an update, that citation and any orphaned items are wiped from the processor registry, which would break the alignment of document and processor state.

fbennett commented 8 years ago

On citationsPre and citationsPost, the processor returns an array of update objects, each with a position, a string, and a citationID. Changing any citation in the document may trigger changes to other citations. For example, if in an author-date style we have a reference to "Smith et al. 2000," with authors Smith, Jones, Doe & Roe, and we add a second reference from 2000 with authors "Smith, Anderson, Doe & Roe", the existing "Smith et al." references may be updated to "Smith, Jones et al." (or to Smith et al. 2000a, depending on the style). The processor will return an array of the citations that require update, and the document-side code needs to work out where to place them.

fbennett commented 8 years ago

Meanwhile, I've more or less figured out how plugins work in tinyMCE, and have sympathy with you over the complexity. It's a big job, with lots of state to maintain (for undo operations), and lots of around-the-end communication between the document in the iframe and the code running in the enclosing page. Small steps. :-)

dsifford commented 8 years ago

Got it. Wooo, that's pretty complex, but doable I suppose.

Re: Ignoring sortedItems -- Sounds good to me. I'm really just trying to figure out what shape the args of processCitationCluster need to be. I'm still not sure where some of this data comes from (and I've spend the last 3 days stepping through the code with devtools!)

I assume I need to provide the exact same shape of data to the function that you do in the example and, given that assumption, I'm not sure how I'd be able to do that if I ignore sortedItems.

Re: TinyMCE being a pain --- Just wait til you dig into it more.. You haven't seen anything yet! I've resorted almost entirely to custom methods since theirs aren't very useful.

dsifford commented 8 years ago

Another question: How do you get citeproc.registry.citationreg to populate? It looks like that contains the data in the shape that I need, but I cannot for the life of me figure out how to get it to make the data it needs.

fbennett commented 8 years ago

The data there is updated when processCitationCluster() is run. Submitting data to that method is the only way to change the registry without breaking things.

dsifford commented 8 years ago

Gotcha.. Hm. Well I'm not sure then how I can call that method the first time then if I have no access to citationByIndex?

Gonna call it a night there for now. But yeah, I'm stuck at figuring out how to get the object with the correct shape to be used as the first arg of processCitationCluster.

fbennett commented 8 years ago

Good night! The first one is built from scratch. The citationID will be empty: it's assigned by the processor, and the unknown citationID in the update return is how you pick out a new item.

A small example of the character of everything-related-to-everything-else-ness that permeates this stuff. On May 20, 2016 11:24, "Derek Sifford" notifications@github.com wrote:

Gotcha.. Hm. Well I'm not sure then how I can call that method the first time then if I have no access to citationByIndex?

Gonna call it a night there for now. But yeah, I'm stuck at figuring out how to get the object with the correct shape to be used as the first arg of processCitationCluster.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/4#issuecomment-220502074

dsifford commented 8 years ago

Does updateItems have any effect on the registry?

What about times when I just need to update the registry (but do not need to make a "citation cluster" (eg, when a reference is added but not yet inserted inline).

dsifford commented 8 years ago

Also, there seems to be quite a lot of circular references in the citeproc object; one example being state.

state references registry which references state which references registry (repeat forever)

dsifford commented 8 years ago

I'm playing around with processCitationCluster and receiving back a response without inline citations...

image

Any ideas?

fbennett commented 8 years ago

It's not elegant in there. On May 21, 2016 02:46, "Derek Sifford" notifications@github.com wrote:

Also, there seems to be quite a lot of circular references in the citeproc object; one example begin state.

state has registry which has state which has registry (repeat forever)

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/4#issuecomment-220672864

dsifford commented 8 years ago

Update Re: Not getting inline citations --- When processCitationCluster calls the internal process_citationCluster function, the sortedItems param is always an empty array. How is this set?

Hard to follow this stuff since most of the processors functions are impure and depend on side effects.

fbennett commented 8 years ago

Yes, I should have said there are three: processCitationCluster(), updateItems(), and updateUncitedItems().

(Plus rebuildProcessorState() of course).

There isn't a reason to use updateItems() in word processor context that I can think of. It's useful when creating standalone bibliographies. (Presumably updateUncitedItems() would do just as well for that, although I've not tried it) On May 21, 2016 02:43, "Derek Sifford" notifications@github.com wrote:

Does updateItems have any effect on the registry?

What about times when I just need to update the registry (but do not need to make a "citation cluster" (eg, when a reference is added but not yet inserted inline).

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/4#issuecomment-220671978

fbennett commented 8 years ago

Note, though, that it needs the full list of item IDs, not just one for update. On May 21, 2016 02:43, "Derek Sifford" notifications@github.com wrote:

Does updateItems have any effect on the registry?

What about times when I just need to update the registry (but do not need to make a "citation cluster" (eg, when a reference is added but not yet inserted inline).

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/4#issuecomment-220671978

fbennett commented 8 years ago

What's the input? On May 21, 2016 03:52, "Derek Sifford" notifications@github.com wrote:

I'm playing around with processCitationCluster and receiving back a response without inline citations...

[image: image] https://cloud.githubusercontent.com/assets/5240018/15438507/7d5070e4-1e9a-11e6-9892-b9cd5b6762c3.png

Any ideas?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/4#issuecomment-220689292

dsifford commented 8 years ago

Found the reason why sortedItems wasn't working: I was giving the function params of slightly the wrong shape...

In this case, this..

{
  citationItems: {
    0: 'the-citation-id'
  },
  properties: {
    noteIndex: 0,
  },
}

rather than this..

{
  citationItems: {
    0: { id: 'the-citation-id' },
  },
  properties: {
    noteIndex: 0,
  },
}

Trying again now...

fbennett commented 8 years ago

citationItems should be array, not object. Processor will query its length. On May 21, 2016 04:39, "Derek Sifford" notifications@github.com wrote:

Found the reason why sortedItems wasn't working: I was giving the function params of slightly the wrong shape...

In this case, this..

{ citationItems: { 0: 'the-citation-id' }, properties: { noteIndex: 0, }, }

rather than this..

{ citationItems: { 0: { id: 'the-citation-id' }, }, properties: { noteIndex: 0, }, }

Trying again now...

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/4#issuecomment-220699836

dsifford commented 8 years ago

That did it!

image

.....slowly but surely we're coming along!

fbennett commented 8 years ago

Yay! Really good to see it clicking. Important progress.

dsifford commented 8 years ago

Thanks again for being so patient and helpful.

I ended the night last night with the inline citation insertion fully working (barely -- lots of edge cases that need to be handled still)

The next tasks that I'll be needing to look into are the following:

  1. What is the exact state (including the shape) that I need to be keeping track of for instances where the processor needs to be rebuilt (eg changing citation styles) or recreated from after saving, leaving, and returning.
  2. What is the series of processor commands that I'll have to run in order to reinstantiate the processor from scratch using saved state JSON data?
  3. How to best handle insertion of inline citations. Right now, it very loosely inserts them using some built-in TinyMCE methods, but there are some problems:
  4. When inserting at the current cursor position, the cursor remains within the span tag of the inserted citation. So inserting and typing is a poor experience because you have to manually click out of it (this is especially apparent when citations are superscript).
  5. Citations can be manually deleted still without the app or processor state being aware. How to best guard against this.
    1. I have no ability to be able to add a citation to an existing set of citations. As of right now, I'm calling this "good" because there are too many other things to get sorted out. But it will require the user to delete a citation block completely and reinsert the entire block of citations in order to append an additional citation (I actually prefer the delete and recreate method over mutating the current state of the citation block so I think I'll probably keep it this way internally even after figuring out what sort of UI should be used to add an additional citation).
    2. Probably a bunch of other things that I'm forgetting...

Let me know your thoughts. Especially on issues 1 and 2.

fbennett commented 8 years ago

Bedtime here, so I'll have to pick up tomorrow. In the meantime...

That's all I can think of for now, hope it makes sense. Will pick up again tomorrow!

dsifford commented 8 years ago

Loading the tinyMCE noneditable plugin and setting an mceNonEditable class on citations looks like a handy solution to the problem at 3.1.

I already added that plugin to the current version of my project. In theory, this would work exactly like I need it to. But here are the two critical problems:

  1. "noneditable" only means that you cannot add or remove text within that tag. It does not mean you can't delete it completely. If the cursor is just to the right of a nonEditable block, hitting backspace wipes it out completely.
  2. I still have yet to figure out how to get TinyMCE to immediately register the nonEditable field. For the current implementation, it takes a few seconds for TinyMCE to become aware of it and, in those few seconds, if the user keeps typing after inserting a citation, that text goes into the nonEditable block and then gets locked in place once the state refreshes. I'm still trying to figure out how to place the cursor outside of that tag once I insert the citation, but I haven't found a very good solution yet. Also, I've tried various methods for refreshing the state immediately (fire, onChange, etc) but none have worked. The documentation really isn't very helpful.

At 3.2(i), do you mean adding items to an existing citation? If so, that should just be a minor refinement. The UI will need to reflect the current items and allow editing, and on submit you just replace the citation's citationItems with one derived from the UI settings, and resubmit to callRegisterCitation().

That's what I thought. Good to know.

About 3.2, stepping across the document citation nodes immediately before setting or editing a citation node seems to be the best (well, the only) way to bring the content of citationByIndex into line with reality. All you need to collect during the scan is the citationID (i.e. the id) of each node, and its note number. For note styles, this will be the citation index plus one; for in-text styles, it will always be 0. The shape of the data is shown in the source code comments.

Good point.

I assume then, say, if the citation at index 2 (of 5) was deleted, and the processor received a list containing indices 0,1,3,4,5 then it would adjust it's internal state by removing the citation at index 2, and adjusting the indices to 0,1,2,3,4?

This may be a silly question: What is a note? Is that the bibliography?

Why does note differ by 1 from in-text? Can't the processor adjust for that internally?

Re: Shape of the data -- It's still a bit nebulous because the processor allows you to pass in an object that implements an interface with these fields, but does not require all of them. It's difficult to determine how, or when, various params within that interface are needed (and where they come from).

serializing the individual citation objects and stashing them on their respective citation nodes

This would be the ideal scenario, but the data that you'd need to serialize in line is huge. I haven't done it yet, but I'd have to imagine that it to be upwards 1000 characters or so.

The issue there is that it makes switching to the code editor of tinymce to fix a wordpress parsing mistake (something that happens often) super difficult because you have to scroll through the looooooooooooooong list of data to get to where you need (for the non-tech-savvy the UX for this would be terrible).

At the day job today, but I'll keep thinking and see if I can try some of your recommendations tomorrow.

fbennett commented 8 years ago

"noneditable" only means that you cannot add or remove text within that tag. It does not mean you can't delete it completely. If the cursor is just to the right of a nonEditable block, hitting backspace wipes it out completely.

That's a feature, I think. The intuitive way to remove a citation is just to delete it. Undo works, so recovery from a miskey is easy. So long as document state is confirmed before a subsequent edit, things will still work. Down the road, it might want a "Refresh" button to fix back-references after a delete, but that's easy enough to implement once the basics are working.

I still have yet to figure out how to get TinyMCE to immediately register the nonEditable field.

Yeah, I wasn't able to reproduce that before picking up on your note. It seems to work fine here.

What is a note? Is that the bibliography?

There are note styles (their counterpart is in-text). The spec doesn't say so explicitly, but they force their references into footnotes, by generating one if the reference is inserted into the text. In a word processor, the note number is sniffed directly from the WP document---it has to be, since users can create their own footnotes, and user-created footnotes might have zero citations or multiple citations. Since tinyMCE doesn't support footnotes natively, we have to spoof them. Where all notes are generated from citations, it is safe to assume (in a note style) that all citations create exactly one footnote, and that each footnote contains exactly one citation. So the note number can be set to the citation index position plus one.

The issue there is that it makes switching to the code editor of tinymce to fix a wordpress parsing mistake (something that happens often) super difficult because you have to scroll through the looooooooooooooong list of data to get to where you need (for the non-tech-savvy the UX for this would be terrible).

Oh. I hadn't thought of that. Agree that it would be pretty bad. It would also be painful to serialize the entire content of citationByIndex in one go each time a citation is edited or added. So ...

Today I'll play with ways of reflecting data in the document, starting with this rough idea:

Something like that anyway. The Zotero integration layer contains similar logic, and I always wondered why all that jiggery-pokery with flagging deletes and whatnot was necessary. I'm getting the picture at last---not a pretty picture, but at least there are reasons for it.

If you have ideas on how to handle this more gracefully, let's discuss---feels like the de facto standard beast is creeping up on us.

dsifford commented 8 years ago

That's a feature, I think. The intuitive way to remove a citation is just to delete it. Undo works, so recovery from a miskey is easy. So long as document state is confirmed before a subsequent edit, things will still work. Down the road, it might want a "Refresh" button to fix back-references after a delete, but that's easy enough to implement once the basics are working.

Yeah, undo works. Haven't had any complaints yet about it so I suppose it'll be fine for now.

Yeah, I wasn't able to reproduce that before picking up on your note. It seems to work fine here.

The reason turned out to be the method I was using wasn't triggering a state refresh of TinyMCE.

So, in other words, switching from this...

const citation = editor.dom.create('span', {id: data[0][2], class: 'abt_cite noselect mceNonEditable'}, data[0][1]);
    editor.selection.getNode().appendChild(citation);

to this...

editor.insertContent(`<span id='${data[0][2]}' class='abt_cite noselect mceNonEditable'>${data[0][1]}</span> `);

solved the issue.

There are note styles (their counterpart is in-text). The spec doesn't say so explicitly, but they force their references into footnotes, by generating one if the reference is inserted into the text. In a word processor, the note number is sniffed directly from the WP document---it has to be, since users can create their own footnotes, and user-created footnotes might have zero citations or multiple citations. Since tinyMCE doesn't support footnotes natively, we have to spoof them. Where all notes are generated from citations, it is safe to assume (in a note style) that all citations create exactly one footnote, and that each footnote contains exactly one citation. So the note number can be set to the citation index position plus one

Lost me completely here. 😖

Re: your thoughts on alternatives to saving citation state within the document. Genius. I hadn't even considered using a "normalized" structure, but that would probably be the best way to go about it. I can save a representation of the data that I need as an Immutable List of Maps and keep it in memory. When the user saves, the immutable List can be converted to standard JSON (using toJS() and then serialized and saved to the database using a hidden text field.

The only downside here is: In order for reusability to be possible, the user would have to take an explicit action to generate the html (as described above) -- I could probably build in an "Export document as" feature down the road to allow exporting to word processors or reference managers.

Curious to see what you come up with when you play around.