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.

Sign up for GitHub

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

Make the image remove option work #3837

Merged
merged 7 commits into from
Nov 23, 2022
Merged

Make the image remove option work #3837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
60080d6
Remove image from innerHTML on remove.
rtibbles Nov 22, 2022
01b4503
Create remove event mechanism for clearing custom span element.
rtibbles Nov 23, 2022
02216e9
Consolidate all blank space wrapping in custom markdown field logic.
rtibbles Nov 23, 2022
d5b981e
Clean up squire keydown handling.
rtibbles Nov 23, 2022
ff87c48
Modify prop values using web component interface rather than
rtibbles Nov 23, 2022
f1f0a5c
Add extra edge case handling for sibling checking.
rtibbles Nov 23, 2022
48a97be
Tweak markdown conversion to prevent addition of excess line breaks.
rtibbles Nov 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 24 additions & 62 deletions ...on/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/MarkdownEditor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import * as Showdown from 'showdown';

import Editor from '@toast-ui/editor';
import debounce from 'lodash/debounce';
import { stripHtml } from 'string-strip-html';

import imageUpload, { paramsToImageFieldHTML } from '../plugins/image-upload';
Expand All @@ -75,16 +74,13 @@
import { registerMarkdownFormulaField } from '../plugins/formulas/MarkdownFormulaField';
import { registerMarkdownImageField } from '../plugins/image-upload/MarkdownImageField';
import { clearNodeFormat, getExtensionMenuPosition } from './utils';
import keyHandlers from './keyHandlers';
import FormulasMenu from './FormulasMenu/FormulasMenu';
import ImagesMenu from './ImagesMenu/ImagesMenu';
import ClickOutside from 'shared/directives/click-outside';

registerMarkdownFormulaField();
registerMarkdownImageField();

const wrapWithSpaces = html => ` ${html} `;

const AnalyticsActionMap = {
Bold: 'Bold',
Italic: 'Italicize',
Expand Down Expand Up @@ -164,7 +160,6 @@
markdown(newMd, previousMd) {
if (newMd !== previousMd && newMd !== this.editor.getMarkdown()) {
this.editor.setMarkdown(newMd);
this.updateCustomNodeSpacers();
this.initImageFields();
}
},
Expand All @@ -182,12 +177,14 @@
const Convertor = tmpEditor.convertor.constructor;
class CustomConvertor extends Convertor {
toMarkdown(content) {
content = showdown.makeMarkdown(content);
content = imagesHtmlToMd(content);
content = formulaHtmlToMd(content);
content = content.replaceAll(' ', ' ');

content = showdown.makeMarkdown(content);
// TUI.editor sprinkles in extra `<br>` tags that Kolibri renders literally
// When showdown has already added linebreaks to render these in markdown
// so we just remove these here.
content = content.replaceAll('<br>', '');

// any copy pasted rich text that renders as HTML but does not get converted
// will linger here, so remove it as Kolibri will render it literally also.
content = stripHtml(content).result;
Expand Down Expand Up @@ -306,35 +303,6 @@
this.keyDownEventListener = this.$el.addEventListener('keydown', this.onKeyDown, true);
this.clickEventListener = this.$el.addEventListener('click', this.onClick);
this.editImageEventListener = this.$el.addEventListener('editImage', this.handleEditImage);

// Make sure all custom nodes have spacers around them.
// Note: this is debounced because it's called every keystroke
const editorEl = this.$refs.editor;
this.updateCustomNodeSpacers = debounce(() => {
editorEl.querySelectorAll('span[is]').forEach(el => {
el.editing = true;
const hasLeftwardSpace = el => {
return (
el.previousSibling &&
el.previousSibling.textContent &&
/\s$/.test(el.previousSibling.textContent)
);
};
const hasRightwardSpace = el => {
return (
el.nextSibling && el.nextSibling.textContent && /^\s/.test(el.nextSibling.textContent)
);
};
if (!hasLeftwardSpace(el)) {
el.insertAdjacentText('beforebegin', '\xa0');
}
if (!hasRightwardSpace(el)) {
el.insertAdjacentText('afterend', '\xa0');
}
});
}, 150);

this.updateCustomNodeSpacers();
},
activated() {
this.editor.focus();
Expand All @@ -358,15 +326,9 @@
* a recommended solution here https://github.com/neilj/Squire/issues/107
*/
onKeyDown(event) {
const squire = this.editor.getSquire();

// Apply squire selection workarounds
this.fixSquireSelectionOnKeyDown(event);

if (event.key in keyHandlers) {
keyHandlers[event.key](squire);
}

// ESC should close menus if any are open
// or close the editor if none are open
if (event.key === 'Escape') {
Expand Down Expand Up @@ -413,8 +375,6 @@
event.preventDefault();
event.stopPropagation();
}

this.updateCustomNodeSpacers();
},
onPaste(event) {
const fragment = clearNodeFormat({
Expand Down Expand Up @@ -507,7 +467,7 @@
const getRightwardElement = selection => getElementAtRelativeOffset(selection, 1);

const getCharacterAtRelativeOffset = (selection, relativeOffset) => {
let { element, offset } = squire.getSelectionInfoByOffset(
const { element, offset } = squire.getSelectionInfoByOffset(
selection.startContainer,
selection.startOffset + relativeOffset
);
Expand All @@ -529,27 +489,31 @@
/\s$/.test(getCharacterAtRelativeOffset(selection, 0));

const moveCursor = (selection, amount) => {
let { element, offset } = squire.getSelectionInfoByOffset(
selection.startContainer,
selection.startOffset + amount
);
if (amount > 0) {
selection.setStart(element, offset);
} else {
selection.setEnd(element, offset);
}
const element = getElementAtRelativeOffset(selection, amount);
selection.setStart(element, 0);
selection.setEnd(element, 0);
return selection;
};

// make sure Squire doesn't delete rightward custom nodes when 'backspace' is pressed
if (event.key !== 'ArrowRight' && event.key !== 'Delete') {
if (isCustomNode(getRightwardElement(selection))) {
const rightwardElement = getRightwardElement(selection);
const leftwardElement = getLeftwardElement(selection);

if (event.key === 'ArrowRight') {
if (isCustomNode(rightwardElement)) {
squire.setSelection(moveCursor(selection, 1));
} else if (spacerAndCustomElementAreRightward(selection)) {
squire.setSelection(moveCursor(selection, 2));
}
}
if (event.key === 'ArrowLeft') {
if (isCustomNode(leftwardElement)) {
squire.setSelection(moveCursor(selection, -1));
} else if (spacerAndCustomElementAreLeftward(selection)) {
squire.setSelection(moveCursor(selection, -2));
}
}
// make sure Squire doesn't get stuck with a broken cursor position when deleting
// elements with `contenteditable="false"` in FireFox
let leftwardElement = getLeftwardElement(selection);
if (event.key === 'Backspace') {
if (selection.startContainer.tagName === 'DIV') {
// This happens normally when deleting from the beginning of an empty line...
Expand Down Expand Up @@ -791,7 +755,6 @@
} else {
let squire = this.editor.getSquire();
squire.insertHTML(formulaHTML);
this.updateCustomNodeSpacers();
}
},
resetFormulasMenu() {
Expand Down Expand Up @@ -876,8 +839,7 @@
const mdImageEl = template.content.firstElementChild;
mdImageEl.setAttribute('editing', true);

// insert non-breaking spaces to allow users to write text before and after
this.editor.getSquire().insertHTML(wrapWithSpaces(mdImageEl.outerHTML));
this.editor.getSquire().insertHTML(mdImageEl.outerHTML);

this.initImageFields();
}
Expand Down
74 changes: 0 additions & 74 deletions ...ration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/keyHandlers.js
View file
Open in desktop

This file was deleted.

8 changes: 6 additions & 2 deletions ...curation/frontend/shared/views/MarkdownEditor/plugins/image-upload/MarkdownImageField.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,12 @@
);
},
handleRemove() {
this.$destroy();
this.$el.parentNode.removeChild(this.$el);
this.editorField.dispatchEvent(
new CustomEvent('remove', {
bubbles: true,
cancelable: true,
})
);
},
handleResize() {
this.resizing = true;
Expand Down
80 changes: 76 additions & 4 deletions ...ntentcuration/frontend/shared/views/MarkdownEditor/plugins/registerCustomMarkdownField.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,54 @@ import Vue from 'vue';
import vueCustomElement from 'vue-custom-element';
import { v4 as uuidv4 } from 'uuid';

const leftwardSpaceRegex = /\s$/;

const leftwardDoubleSpaceRegex = /\s\s$/;

const hasLeftwardSpace = el => {
return (
// Has a previous sibling
el.previousSibling &&
// Which has text content
el.previousSibling.textContent &&
// The text content has white space right before this element
leftwardSpaceRegex.test(el.previousSibling.textContent) &&
// And either this sibling doesn't have a previous sibling
(!el.previousSibling.previousSibling ||
// Or it doesn't have a hasAttribute function
typeof el.previousSibling.previousSibling.hasAttribute !== 'function' ||
// Or the previous sibling is not another custom field
!el.previousSibling.previousSibling.hasAttribute('is') ||
// Or the previous sibling has two white spaces, one for each
// of the custom fields on either side.
leftwardDoubleSpaceRegex.test(el.previousSibling.textContent))
);
};

const rightwardSpaceRegex = /^\s/;

const rightwardDoubleSpaceRegex = /^\s\s/;

const hasRightwardSpace = el => {
return (
// Has a next sibling
el.nextSibling &&
// Which has text content
el.nextSibling.textContent &&
// The text content has white space right after this element
rightwardSpaceRegex.test(el.nextSibling.textContent) &&
// And either this sibling doesn't have a next sibling
(!el.nextSibling.nextSibling ||
// Or it doesn't have a hasAttribute function
typeof el.nextSibling.nextSibling.hasAttribute !== 'function' ||
// Or the next sibling is not another custom field
!el.nextSibling.nextSibling.hasAttribute('is') ||
// Or the next sibling has two white spaces, one for each
// of the custom fields on either side.
rightwardDoubleSpaceRegex.test(el.nextSibling.textContent))
);
};

export default VueComponent => {
const dashed = camel => camel.replace(/([a-zA-Z])(?=[A-Z])/g, '$1-').toLowerCase();
const name = dashed(VueComponent.name);
Expand All @@ -15,10 +63,10 @@ export default VueComponent => {
vueInstanceCreatedCallback() {
// by default, `contenteditable` will be false
this.setAttribute('contenteditable', Boolean(VueComponent.contentEditable));

const id = `markdown-field-${uuidv4()}`;
// a hack to prevent squire from merging custom element spans
// see here: https://github.com/nhn/tui.editor/blob/master/libs/squire/source/Node.js#L92-L101
this.classList.add(`markdown-field-${uuidv4()}`);
this.classList.add(id);

// pass innerHTML of host element as the `markdown` property
this.observer = new MutationObserver(mutations => {
Expand All @@ -35,14 +83,38 @@ export default VueComponent => {
this.innerHTML = textNodesRemoved.map(n => n.nodeValue).join();
} else {
// otherwise, pass the innerHTML to inner Vue component as `markdown` prop
this.getVueInstance().markdown = this.innerHTML;
this.markdown = this.innerHTML;
}
});
});
this.observer.observe(this, { characterData: true, childList: true });

this.addEventListener('remove', () => {
if (hasLeftwardSpace(this)) {
this.previousSibling.textContent = this.previousSibling.textContent.replace(
leftwardSpaceRegex,
''
);
}
if (hasRightwardSpace(this)) {
this.nextSibling.textContent = this.nextSibling.textContent.replace(
rightwardSpaceRegex,
''
);
}
this.parentNode.removeChild(this);
});

this.editing = true;

if (!hasLeftwardSpace(this)) {
this.insertAdjacentText('beforebegin', '\xa0');
}
if (!hasRightwardSpace(this)) {
this.insertAdjacentText('afterend', '\xa0');
}
// initialize the `markdown` property
this.getVueInstance().$root.markdown = this.innerHTML;
this.markdown = this.innerHTML;
},
shadowCss: VueComponent.shadowCSS,
shadow: true,
Expand Down