From 347c9c4aa5e3392c74c0242d8672e9737f949492 Mon Sep 17 00:00:00 2001 From: Tadhg O'Higgins Date: Wed, 7 Feb 2018 10:18:59 -0800 Subject: [PATCH 1/4] WIP checkin. Working end-to-end link handling. --- api/document/js/menu.ts | 37 ++++++++++++++++++++++++++++++++ api/document/js/parse-doc.ts | 1 + api/document/js/schema.ts | 14 ++++++++++-- api/document/js/serialize-doc.ts | 6 ++++-- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/api/document/js/menu.ts b/api/document/js/menu.ts index 8ae306efc..dc6873d25 100644 --- a/api/document/js/menu.ts +++ b/api/document/js/menu.ts @@ -1,3 +1,4 @@ +import { toggleMark } from 'prosemirror-commands'; import { menuBar, undoItem, redoItem, MenuItem, MenuItemSpec } from 'prosemirror-menu'; import { JsonApi } from './Api'; @@ -9,6 +10,7 @@ import { makeSaveThenXml, } from './commands'; import icons from './icons'; +import schema from './schema'; function makeButton(content) { return new MenuItem({ @@ -38,6 +40,7 @@ export default function menu(api: JsonApi) { run: appendParagraphNear, title: 'Append paragraph', }), + linkItem(schema.marks.external_link), makeButton({ icon: icons.newBulletList, run: appendBulletListNear, @@ -62,3 +65,37 @@ export default function menu(api: JsonApi) { ], }); } + +function markActive(state, type) { + const { from, $from, to, empty } = state.selection; + if (empty) { + return type.isInSet(state.storedMarks || $from.marks()); + } + return state.doc.rangeHasMark(from, to, type); +} + +function linkItem(markType) { + return new MenuItem({ + class: 'menuitem-clickable', + // These defaults are needed due to a doc issue. See + // https://github.com/ProseMirror/prosemirror-menu/issues/15 + css: '', + execEvent: 'mousedown', + title: 'Add or remove link', + label: 'A', + active(state) { return markActive(state, markType); }, + enable(state) { return !state.selection.empty; }, + run(state, dispatch, view) { + if (markActive(state, markType)) { + toggleMark(markType)(state, dispatch); + return true; + } + // We need a replacement for prompt here. + toggleMark(schema.marks.external_link, { + href: prompt('URL', 'not this'), + })(view.state, view.dispatch); + view.focus(); + return true; + }, + }); +} diff --git a/api/document/js/parse-doc.ts b/api/document/js/parse-doc.ts index a9a4f9aa5..eedcd479c 100644 --- a/api/document/js/parse-doc.ts +++ b/api/document/js/parse-doc.ts @@ -99,4 +99,5 @@ const NODE_TYPE_CONVERTERS: NodeConverterMap = { const CONTENT_TYPE_CONVERTERS = { unimplementedMark: content => factory.unimplementedMark(content), + external_link: content => factory.external_link(content.href), }; diff --git a/api/document/js/schema.ts b/api/document/js/schema.ts index 2c5cce174..84c3b743a 100644 --- a/api/document/js/schema.ts +++ b/api/document/js/schema.ts @@ -79,17 +79,25 @@ const schema = new Schema({ }, toDOM(node) { const nodeType = node.attrs.data.node_type || '[no-node-type]'; - return ['div', { class: 'unimplemented' }, nodeType]; + return ['div', { class: 'unimplemented whatever' }, nodeType]; }, }, ...listSchemaNodes, }, marks: { + external_link: { + attrs: { + href: {}, + }, + toDOM(data) { + return ['a', { class: `external-link`, href: data.attrs.href }]; + }, + }, unimplementedMark: { attrs: { data: {}, // will hold unrendered content }, - toDOM: () => ['span', { class: 'unimplemented' }], + toDOM: data => ['span', { class: `unimplemented` }], }, }, }); @@ -151,6 +159,8 @@ export const factory = { schema.nodes.policy.create({}, children || []), sec: (children?: Node[] | Fragment) => schema.nodes.sec.create({}, children || []), + external_link: (href: string) => + schema.marks.external_link.create({ href }), unimplementedMark: (original: any) => schema.marks.unimplementedMark.create({ data: original }), unimplementedNode: (original: any) => diff --git a/api/document/js/serialize-doc.ts b/api/document/js/serialize-doc.ts index 54c7ffb91..e2d699438 100644 --- a/api/document/js/serialize-doc.ts +++ b/api/document/js/serialize-doc.ts @@ -94,8 +94,10 @@ function defaultNodeConverter(node: Node): ApiNode { } const MARK_CONVERTERS = { - unimplementedMark: node => - apiFactory.content(node.type.name, node.attrs.data), + unimplementedMark: mark => + apiFactory.content(mark.type.name, mark.attrs.data), + external_link: mark => + apiFactory.content(mark.type.name, { href: mark.attrs.href }), }; From b5ff8b9969674c4bacf1a27d0358d56f9e7d39fd Mon Sep 17 00:00:00 2001 From: Tadhg O'Higgins Date: Thu, 15 Feb 2018 11:19:53 -0800 Subject: [PATCH 2/4] WIP checkin. Add href as optional in ApiContent schema, first pass at reasonable parsing tests for external links. --- api/document/js/Api.ts | 1 + api/document/js/__tests__/menu.test.ts | 2 + api/document/js/__tests__/parse-doc.test.ts | 47 ++++++++++++++++++++- api/document/js/menu.ts | 8 ++-- 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/api/document/js/Api.ts b/api/document/js/Api.ts index dd8defacf..613c46646 100644 --- a/api/document/js/Api.ts +++ b/api/document/js/Api.ts @@ -34,6 +34,7 @@ export interface ApiContent { inlines: ApiContent[]; text: string; footnote_node?: ApiNode; + href?: string; } export class Api { diff --git a/api/document/js/__tests__/menu.test.ts b/api/document/js/__tests__/menu.test.ts index d3c4f88f3..0ca250f04 100644 --- a/api/document/js/__tests__/menu.test.ts +++ b/api/document/js/__tests__/menu.test.ts @@ -21,11 +21,13 @@ describe('menu', () => { 'Undo last change', 'Redo last undone change', 'Append paragraph', + 'Add or remove link', 'Append bullet list', 'Append ordered list', 'Save document', 'Save document then edit as XML', ]; + expect(menuItemTitles.length).toEqual(expectedItemTitles.length); expectedItemTitles.forEach((title) => { expect(menuItemTitles.includes(title)); }); diff --git a/api/document/js/__tests__/parse-doc.test.ts b/api/document/js/__tests__/parse-doc.test.ts index 4f3e305c0..135ab5bf5 100644 --- a/api/document/js/__tests__/parse-doc.test.ts +++ b/api/document/js/__tests__/parse-doc.test.ts @@ -1,6 +1,6 @@ import axios from 'axios'; -import { ApiContent } from '../Api'; +import { ApiContent, ApiNode } from '../Api'; import parseDoc, { convertContent } from '../parse-doc'; import { apiFactory } from '../serialize-doc'; import schema from '../schema'; @@ -117,6 +117,51 @@ describe('parseDoc()', () => { expect(lis[1].content.child(1).type.name).toBe('para'); }); + it('loads links', () => { + const node: ApiNode = { + node_type: 'para', + content: [ + { + content_type: '__text__', + text: 'Initial ', + inlines: [], + }, + { + content_type: 'external_link', + href: 'http://example.org', + text: 'content.', + inlines: [{ content_type: '__text__', text: 'content.', inlines: [] }], + }, + + ], + children: [], + }; + + const result = parseDoc(node); // The top-level node. + expect(result.type.name).toBe('para'); + expect(result.content.size).toEqual('Initial content.'.length + 2); // Node boundaries add two. + expect(result.childCount).toEqual(1); // One inner node. + + const paraText = result.child(0); // The inner paraText node. + expect(paraText.content.size).toEqual('Initial content.'.length); + expect(paraText.type.name).toEqual('paraText'); + expect(paraText.childCount).toEqual(2); // The two inline children of the inner node. + + const plainText = paraText.child(0); // The first inline child, plain text. + expect(plainText.type.name).toEqual('text'); + expect(plainText.text).toEqual('Initial '); + + const externalLink = paraText.child(1); // The second inline child, containing the link. + expect(externalLink.type.name).toEqual('text'); + expect(externalLink.text).toEqual('content.'); + expect(externalLink.marks.length).toEqual(1); // Just one link. + + const mark = externalLink.marks[0]; // The link markup. + expect(mark.type.name).toEqual('external_link'); + expect(mark.attrs.href).toEqual('http://example.org'); + + }); + describe('unimplementedNode', () => { it('saves original data', () => { const node = { diff --git a/api/document/js/menu.ts b/api/document/js/menu.ts index dc6873d25..0087d0af6 100644 --- a/api/document/js/menu.ts +++ b/api/document/js/menu.ts @@ -33,14 +33,14 @@ export default function menu(api: JsonApi) { // prosemirror-menu. For more details, see: // // https://github.com/ProseMirror/prosemirror-menu/issues/12 - undoItem as any as MenuItem, - redoItem as any as MenuItem, + undoItem as any as MenuItem, // title: 'Undo last change' + redoItem as any as MenuItem, // title: 'Redo last undone change' makeButton({ label: 'P', run: appendParagraphNear, title: 'Append paragraph', }), - linkItem(schema.marks.external_link), + linkItem(schema.marks.external_link), // title: 'Add or remove link' makeButton({ icon: icons.newBulletList, run: appendBulletListNear, @@ -58,8 +58,8 @@ export default function menu(api: JsonApi) { }), makeButton({ label: 'Save then XML', - title: 'Save document then edit as XML', run: makeSaveThenXml(api), + title: 'Save document then edit as XML', }), ], ], From 434044c13011900e70018f8906f1e11369e644cc Mon Sep 17 00:00:00 2001 From: Tadhg O'Higgins Date: Thu, 22 Feb 2018 13:55:13 -0800 Subject: [PATCH 3/4] Last tweaks/notes on external link stuff. --- api/document/js/__tests__/parse-doc.test.ts | 3 +++ api/document/js/menu.ts | 27 ++++++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/api/document/js/__tests__/parse-doc.test.ts b/api/document/js/__tests__/parse-doc.test.ts index 135ab5bf5..444674d51 100644 --- a/api/document/js/__tests__/parse-doc.test.ts +++ b/api/document/js/__tests__/parse-doc.test.ts @@ -118,6 +118,9 @@ describe('parseDoc()', () => { }); it('loads links', () => { + // This checks link parsing but doesn't check any of the interaction + // involved, such as making sure a prompt actually pops up in response + // to a button click. const node: ApiNode = { node_type: 'para', content: [ diff --git a/api/document/js/menu.ts b/api/document/js/menu.ts index 0087d0af6..628a79c15 100644 --- a/api/document/js/menu.ts +++ b/api/document/js/menu.ts @@ -74,6 +74,20 @@ function markActive(state, type) { return state.doc.rangeHasMark(from, to, type); } +function externalLink(state, dispatch, view, markType) { + // This function might belong in ./commands + if (markActive(state, markType)) { + toggleMark(markType)(state, dispatch); + return true; + } + // We need a replacement for prompt here. + toggleMark(schema.marks.external_link, { + href: prompt('URL', 'URL: '), + })(view.state, view.dispatch); + view.focus(); + return true; +} + function linkItem(markType) { return new MenuItem({ class: 'menuitem-clickable', @@ -85,17 +99,6 @@ function linkItem(markType) { label: 'A', active(state) { return markActive(state, markType); }, enable(state) { return !state.selection.empty; }, - run(state, dispatch, view) { - if (markActive(state, markType)) { - toggleMark(markType)(state, dispatch); - return true; - } - // We need a replacement for prompt here. - toggleMark(schema.marks.external_link, { - href: prompt('URL', 'not this'), - })(view.state, view.dispatch); - view.focus(); - return true; - }, + run(state, dispatch, view) { return externalLink(state, dispatch, view, markType); }, }); } From a6c430454c7906f7fc3be1272fb247f7aad79e59 Mon Sep 17 00:00:00 2001 From: CM Lubinski Date: Wed, 28 Feb 2018 08:50:44 -0500 Subject: [PATCH 4/4] Resolve warning when running tests. We (asynchronously) check an attribute of `data` later, so we need to ensure it exists if we want to avoid warnings in our tests. --- api/document/js/__tests__/Api.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/api/document/js/__tests__/Api.test.ts b/api/document/js/__tests__/Api.test.ts index 6f590f539..ccb6dfd73 100644 --- a/api/document/js/__tests__/Api.test.ts +++ b/api/document/js/__tests__/Api.test.ts @@ -23,6 +23,7 @@ const api = new Api({ }); describe('fetch()', () => { + axios.get = jest.fn(async () => ({ data: '' })); it('passes the correct args', () => { api.fetch(); expect(axios.get).toHaveBeenCalledWith(