TRY: Footnotes #2818

tg-ephox · 2017-09-28T05:08:26Z

Description

fixes #1890

Todo:

Footnote Entry

Footnotes Block

create new block
single use block
always position footnote block at end
enable footnotes in FormatToolbar when footnote block added

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows has proper inline documentation.

tg-ephox · 2017-09-28T05:16:54Z

This is very much a work in progress. I just wanted some feedback on the approach. Currently footnotes are stored as attributes on the Footnotes block which is read-only. The biggest hurdle will be how to re-number footnotes when a new footnote is added (or one removed). Do we want to be iterating each node in every block (after the inserted foonote) to update the superscript footnote link? This can probably be made simpler by storing a mapping between footnoteId and blockId, to say where a footnote has been used...

tg-ephox · 2017-09-29T03:42:41Z

Hey @jasmussen @karmatosed can I pls get some feedback on this? If you're happy with the direction i'll keep going on Tuesday (public holiday on Monday in Australia!! ☺️).

jasmussen · 2017-09-29T07:32:14Z

Nice work! Looks like generally the right direction.

A few quick questions, though:

Is it possible to insert the footnote block automatically at the bottom of the list, as soon as you create the first footnote?
Is it possible to actually preview the content written in the footnote block? In my test I just saw a "Footnotes" header in the block
Alternatively, is there a way to preview the footnotes at the bottom, without this being an actual block that you can move around?

Curious what @aduth thinks also.

tg-ephox · 2017-09-29T07:38:28Z

@jasmussen I was thinking the opposite might be better in regards to inserting the Footnotes block, you add the Footnotes block and this will add the Footnotes formatting option to the block toolbar? (this block will need to be single use and not moveable). The content is currently shown in the block in the editor but you need to blur the Paragraph Block, as currently content is only synced to redux state on focusout (there's another PR for this!)

jasmussen · 2017-09-29T07:44:25Z

this will add the Footnotes formatting option to the block toolbar

This is a good point, the contents of the footnotes need to be rich text.

I do think the experience would be best if the block that holds the footnotes were inserted or appeared automatically (and also disappeared automatically if you removed all footnotes). Anything else is likely to be undiscoverable.

tg-ephox · 2017-09-29T07:47:59Z

@jasmussen sounds good I will make this change. The footnote content is currently just text (textarea)...

jasmussen · 2017-09-29T07:50:04Z

Impressive, wasn't sure if the auto insertion/removal would be easy or hard. But it does seem like the best experience, that the two are connected directly.

aduth

I know the mechanisms for extensibility are not yet well-developed, but with these changes, we are hard-coding awareness of footnotes into:

Block registration
Formatting toolbar
Editable
Editor block rendering

This is not very maintainable, and honestly I'd prefer we go the route of introducing proper extensibility before considering this if this is what's necessary to implement footnotes.

aduth · 2017-09-29T18:31:40Z

blocks/editable/format-toolbar/index.js

@@ -56,6 +58,10 @@ class FormatToolbar extends Component {
 		this.onChangeLinkValue = this.onChangeLinkValue.bind( this );
 		this.toggleLinkSettingsVisibility = this.toggleLinkSettingsVisibility.bind( this );
 		this.setLinkTarget = this.setLinkTarget.bind( this );
+
+		this.addFootnote = this.addFootnote.bind( this );


Related: #2474

This is begging to be extensible behavior, not engrained into the toolbar itself.

aduth · 2017-09-29T18:34:39Z

blocks/editable/index.js

+			} else if ( format === 'footnote' ) {
+				const footnoteId = uuid();
+				this.footnotes[ footnoteId ] = formatValue.text;
+				this.editor.selection.setContent( `<a href="#footnote-${ footnoteId }" data-footnote-id="${ footnoteId }" contenteditable="false"><sup>[*]</sup></a>` );


This is being persisted into the saved content of the post, which is problematic because we don't want/care for this editor-specific attribute to be present on the front-end of the site. Also, it's causing React to log a warning into the developer console.

aduth · 2017-09-29T18:43:02Z

blocks/library/footnotes/index.js

+registerBlockType( 'core/footnotes', {
+	title: __( 'Footnotes' ),
+
+	icon: 'button',


Guessing this block could use the once: true flag (see also: "More" block)

aduth · 2017-09-29T18:43:21Z

blocks/library/footnotes/index.js

@@ -0,0 +1,35 @@
+import { mapKeys } from 'lodash';


/** * External dependencies */

aduth · 2017-09-29T18:43:26Z

blocks/library/footnotes/index.js

+ */
+import { __ } from '@wordpress/i18n';
+
+import { registerBlockType } from '../../api';


/** * Internal dependencies */

aduth · 2017-09-29T18:44:47Z

blocks/library/paragraph/index.js

 						setAttributes( {
-							content: nextContent,
+							content: nextContent.content,
+							footnotes: nextContent.footnotes,


This is causing paragraph blocks to include an empty footnotes object in comment serialized block attributes whenever the text is changed (regardless of adding footnotes). We should avoid this being included for paragraphs which aren't using footnotes.

…ble)

tg-ephox · 2017-10-05T06:57:43Z

@aduth this is still very much a work in progress... Would like some feedback on the API to add formatters to Editable...

gutenberg/blocks/library/paragraph/index.js

Line 109 in ece64ee

this.footnoteFormatter = {

anna-harrison · 2017-10-06T04:13:51Z

@jasmussen : can we just check - with the proposal to auto insert/remove the footnote block, this assumes that the "Insert Footnote" button is always visible in the toolbar. Is this OK?

jasmussen · 2017-10-06T07:40:58Z

@annaephox

can we just check - with the proposal to auto insert/remove the footnote block, this assumes that the "Insert Footnote" button is always visible in the toolbar. Is this OK?

Yes, that's fine.

We are separaretly ( #2898 ) tracking how to improve many buttons in the quick toolbar, and worst case scenario we can look at moving the button to the inspector sidebar and call it an "advanced feature". This shouldn't block us for now! 👍 👍

anna-harrison · 2017-10-12T02:29:46Z

@aduth: could you please have a quick look at ^ as @tg-ephox is stuck. Thx

aduth · 2017-10-12T20:31:42Z

blocks/library/paragraph/index.js

+
+			this.footnotes = this.props.attributes.footnotes || {};
+
+			this.footnoteFormatter = {


This seems to require a significant amount of effort on the part of the block implementer to track the state of the active node in TinyMCE and apply formatting.

If I understand correctly the needs around identifying and applying formats / tokens, there's a few things which would be ideal:

A reasonable API in TinyMCE to define formats / token types, how they’re identified & applied, maybe emitting specific events when the selection becomes "active" for this type (to integrate with toolbar)

Define a set of built-in / global formats: bold, italic, strikethrough, footnotes

Managed within Editable, not surfaced to block implementer

If possible, expose a simple-to-use prop on Editable for custom formats

So extending the bold, italic idea, one could enable a footnotes format on Editable via formattingControls={ [ 'bold', 'footnotes' ] }.

It seems like there could be some overlap here with how TinyMCE enables toolbar buttons to be defined with a stateSelector property that handles much of the work around determining whether a button should be active or not.

danielbachhuber · 2018-03-30T13:23:02Z

Closing in favor of the existing issue (#1890). We can pull what we need from this PR when that moves forward.

Initial commit

2388afe

tg-ephox added the [Status] In Progress Tracking issues with work in progress label Sep 28, 2017

tg-ephox self-assigned this Sep 28, 2017

tg-ephox requested a review from aduth September 28, 2017 05:18

Detect footnote deletion

26247a2

aduth reviewed Sep 29, 2017

View reviewed changes

Create API to add formatters to Editable (remove footnotes from Edita…

ece64ee

…ble)

Merge branch 'master' into try/1890-footnotes

a9a9108

aduth reviewed Oct 12, 2017

View reviewed changes

danielbachhuber closed this Mar 30, 2018

danielbachhuber deleted the try/1890-footnotes branch March 30, 2018 13:23

androb mentioned this pull request Mar 30, 2018

Add Footnotes Support #1890

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TRY: Footnotes #2818

TRY: Footnotes #2818

tg-ephox commented Sep 28, 2017 •

edited

Loading

tg-ephox commented Sep 28, 2017 •

edited

Loading

tg-ephox commented Sep 29, 2017

jasmussen commented Sep 29, 2017

tg-ephox commented Sep 29, 2017 •

edited

Loading

jasmussen commented Sep 29, 2017

tg-ephox commented Sep 29, 2017 •

edited

Loading

jasmussen commented Sep 29, 2017

aduth left a comment

aduth Sep 29, 2017

aduth Sep 29, 2017

aduth Sep 29, 2017

aduth Sep 29, 2017

aduth Sep 29, 2017

aduth Sep 29, 2017

tg-ephox commented Oct 5, 2017

anna-harrison commented Oct 6, 2017

jasmussen commented Oct 6, 2017

anna-harrison commented Oct 12, 2017 •

edited

Loading

aduth Oct 12, 2017

danielbachhuber commented Mar 30, 2018


		this.footnotes = this.props.attributes.footnotes \|\| {};

		this.footnoteFormatter = {

TRY: Footnotes #2818

TRY: Footnotes #2818

Conversation

tg-ephox commented Sep 28, 2017 • edited Loading

Description

Todo:

Footnote Entry

Footnotes Block

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

tg-ephox commented Sep 28, 2017 • edited Loading

tg-ephox commented Sep 29, 2017

jasmussen commented Sep 29, 2017

tg-ephox commented Sep 29, 2017 • edited Loading

jasmussen commented Sep 29, 2017

tg-ephox commented Sep 29, 2017 • edited Loading

jasmussen commented Sep 29, 2017

aduth left a comment

Choose a reason for hiding this comment

aduth Sep 29, 2017

Choose a reason for hiding this comment

aduth Sep 29, 2017

Choose a reason for hiding this comment

aduth Sep 29, 2017

Choose a reason for hiding this comment

aduth Sep 29, 2017

Choose a reason for hiding this comment

aduth Sep 29, 2017

Choose a reason for hiding this comment

aduth Sep 29, 2017

Choose a reason for hiding this comment

tg-ephox commented Oct 5, 2017

anna-harrison commented Oct 6, 2017

jasmussen commented Oct 6, 2017

anna-harrison commented Oct 12, 2017 • edited Loading

aduth Oct 12, 2017

Choose a reason for hiding this comment

danielbachhuber commented Mar 30, 2018

tg-ephox commented Sep 28, 2017 •

edited

Loading

tg-ephox commented Sep 28, 2017 •

edited

Loading

tg-ephox commented Sep 29, 2017 •

edited

Loading

tg-ephox commented Sep 29, 2017 •

edited

Loading

anna-harrison commented Oct 12, 2017 •

edited

Loading