Conversation
harbournick
left a comment
There was a problem hiding this comment.
See my comment here. LMK if I can clarify anything but essentially we just need to move the logic to work inside the editor, rather than in the docx processor.
| return regex.test(text); | ||
| }; | ||
|
|
||
| export const extractFillableParts = (text) => { |
There was a problem hiding this comment.
I like the idea here to process-on-import, however I think only a subset of SuperDoc customers will want to detect mustaches and convert to fields (some may not use fields at all) so this becomes an optional process (and thus should be taken out of SuperConverter which is lower-level docx converting specifically).
we already have the 'annotations: true' key in the config, we can use this to determine wether to run this at all or not. And, preferably we should do it here: https://github.com/Harbour-Enterprises/SuperDoc/blob/44a6db428cd23855a891cad50f15a79bb4acb68a/packages/super-editor/src/core/Editor.js#L1005 (post conversion, pre prose-mirror rendering)
In fact, the function will look a lot like prepareCommentsForImport() I'm guessing in this case, and we can just optionally run it if the annotations flag is on
There was a problem hiding this comment.
@artem-harbour can you review this one too? I'm curious what you think. Organizationally, something about processing text in the converter rather than strictly processing docx nodes doesn't seem quite right. Mixing of concerns here. Ultimately this could be even better as a plugin, so that even after import if a user types {{ user_name }} it can also be dynamically converted to an annotation. But I'm fine with that being some future state once we learn more about how this feature fits into SuperDoc.
There was a problem hiding this comment.
I agree with you @harbournick that the processing of text in the converter does not seem quite right.
Here are some of my thoughts:
- This should be optional and enabled/disabled via config. This won't be used by everyone, and regex can slow down processing significantly, especially for large documents.
prepareDocumentForImport- this seems to be a more correct place for post processing right inside PM.
Here we can replace the necessary text with annotations or whatever we need.
Something like this.
const annotation = editor.extensionService.schema.nodes.fieldAnnotation.create({}, null, null);
const tr = editor.state.tr;
tr.replaceWith(2, 7, annotation); // replace for needed positions
editor.view.dispatch(tr);The only thing is, we need the positions of specific words to replace, not text nodes. Here we can probably use search-replace module (it supports regex), but not sure.
-
The plugin sounds like an even smarter solution that will make replacements automatically, but I think it's not needed now and will only complicate things.
-
I also think that not every feature we need should be inside superdoc repo. Ideally, I think it should be an extension that is in the webapp and is passed to SD via the config (only the necessary changes are made to the SD to support this).
| attrs.highlighted = false; | ||
| attrs.type = "text"; | ||
| return { | ||
| type: 'fieldAnnotation', |
There was a problem hiding this comment.
fieldAnnotation - this node was not designed for this feature, but probably there will be no issue with it. @harbournick what do you think?
| * } | ||
| * ]) | ||
| */ | ||
| replaceWithFieldAnnotation: |
There was a problem hiding this comment.
I think the function signature could be improved here, it's very much tied to the field name now. Although these don't necessarily have to be fields, just positions.
Example
{
replaceWithFieldAnnotation:
(positions) =>
({ editor, dispatch, state, tr }) => {
if (!dispatch) return true;
tr.setMeta('fieldAnnotationReplace', true);
positions.forEach((position) => {
let { from, to, attrs } = position;
let { schema } = editor;
let newPosFrom = tr.mapping.map(from);
let newPosTo = tr.mapping.map(to);
let node = schema.nodes[this.name].create({...attrs}, null, null);
tr.replaceWith(newPosFrom, newPosTo, node);
});
return true;
},
}| }, | ||
|
|
||
| /** | ||
| * Replace field annotation. |
There was a problem hiding this comment.
Replace positions with field annotations.
+@param description.
No description provided.