Ckeditor5: Consistent naming strategy for buttons and plugins

[Reinmar]

Buttons

Verb + noun (optional):

• insertTable,
• linkImage,
• selectAll,
• mergeTableCells,
• exportWord.

Noun + verb:

• imageUpload,
• imageInsert.

Noun (usually, a feature name):

• mediaEmbed,
• bold,
• paragraph,
• heading,
• restrictedEditing,
• tableCellProperties.

Observation and proposal

IMO, the "noun + verb" category is an inconsequence. 

Proposal

:warning:  I'd rename both plugins to verb + noun (but maintain backward compat by having aliases).

Plugins

Feature name:

• Bold,
• Italic,
• MediaEmbed,
• Heading,
• Paragraph,
• Clipboard,
• Image,
• SpecialCharacters,
• Table,
• Widget.

Feature name + sub-feature:

• HeadingButtons,
• ImageResize,
• ImageUpload,
• ImageResize,
• ListStyle,
• SpecialCharactersArrows, ...
• TableClipboard,
• TableMouse,
• TableProperties,
• TableCellProperties,
• WidgetResize.

Exceptions:

• AutoLink
• TodoList
• Input and Delete (as opposed to TypingInput and TypingDelete).

Observations

We're quite consistent here. There are either [FeatureName] plugins or [FeatureName][SubFeature] ones. There are some exceptions, but I'd say reasonable.

More importantly, though, we can see that if the [SubFeature] part is a verb, the button name diverges from the plugin name. A special case here is insertTable vs Table and in some sense insertImage if it was registered by Image as proposed in #8002. I don't see a problem here, though. I don't think we need to have plugin == button names equality.

tl;dr: I don't think we need to change anything here. We should hace [FatureName]  and  [FeatureName][SubFeature] plugins with some room for exceptions where it makes sense.

---

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

[Reinmar]

TODO:

• commands,
• ADR,
• section in the naming convention guide.

[jodator]

IIRC - we had imageUpload and then we've added insertImage to have it "consistent". However, it looks odd.

For the UI button in #8002 it could make sense to have image button since it handles image related things.

And for this case table might be also acceptable vs insertTable as other features do not have insert (I can think of 'mediaEmbedd' only right now).

[oleq]

IMO, the "noun + verb" category is an inconsequence.

OTOH it allows grouping button names by features (feature->action: imageDoThis, imageDoThat) that looks good and could make sense for integrators. 

But given the popularity of verb+noun across the project, I agree it's an inconsequence and this should be fixed. Besides, it also does not work in terms of natural language (_insert what_ vs. _what insert_).

[jodator]

As discussed on the meeting :+1: for the change in image buttons and to describe the naming convention.

[Reinmar]

We should change the names in this iteration because we introduced the new imageInsert button (which is a part of a breaking change) and it'd be good to not change its name soon after.

We do however have to remember to fix those names in the changelog as it currently holds the notion of imageInsert :D

[Reinmar]

 I reviewed command names and these are trickier. There are more types of their names. However, again _some_ of the image commands stand out. Namely, I'd fix these names: imageInsert, imageResize, imageUpload, todoListCheck. These are the only 4 commands that have noun+verb structure. I'd rename them to insertImage (matches the new button name), resizeImage, uploadImage (matches the new button name) and checkTodoList.

Feature name (noun-based)

Note: Many of these names could also be moved to the next category as they are both nouns and verbs. I moved there only link and undo as these have their verb-based counterparts: unlink and undo.

• fontColor,
• fontBackgroundColor,
• shiftEnter,
• enter,
• blockQuote,
• bold,
• paragraph,
• heading,
• numberedList,
• bulletedList,
• mediaEmbed,
• codeBlock,
• alignment,
• code,
• underline,
• strikethrough,
• superscript,
• subscript,
• highlight,
• fontFamily,
• fontSize,
• pageBreak,
• horizontalLine,
• todoList,
• italic,

Feature-related (verb-based)

• selectAll,
• undo,
• redo,
• forwardDelete,
• delete,
• mention,
• removeFormat,
• indent,
• outdent,
• link,
• unlink,

Feature + sub-feature (noun)

• imageTextAlternative,
• imageStyle,
• tableBorderColor,
• tableBorderStyle,
• tableBorderWidth,
• tableAlignment,
• tableWidth,
• tableHeight,
• tableBackgroundColor,
• tableCellBorderStyle,
• tableCellBorderColor,
• tableCellBorderWidth,
• tableCellHorizontalAlignment,
• tableCellWidth,
• tableCellHeight,
• tableCellPadding,
• tableCellBackgroundColor,
• tableCellVerticalAlignment,

Verb

• insertParagraph,
• insertText (previously: input),
• indentList,
• outdentList,
• insertTable,
• insertTableRowAbove,
• insertTableRowBelow,
• insertTableColumnLeft,
• insertTableColumnRight,
• removeTableRow,
• removeTableColumn,
• splitTableCellVertically,
• splitTableCellHorizontally,
• mergeTableCells,
• mergeTableCellRight,
• mergeTableCellLeft,
• mergeTableCellDown,
• mergeTableCellUp,
• setTableColumnHeader,
• setTableRowHeader,
• selectTableRow,
• selectTableColumn,
• indentCodeBlock,
• outdentCodeBlock,
• indentBlock,
• outdentBlock,

Feature + action (verb)

• imageInsert,
• imageResize,
• imageUpload,
• todoListCheck,

[Reinmar]

My biggest issue with all this is that inside the image package now we'd have:

• insertImage,
• uploadImage,
• imageTextAlternative,
• imageStyle

But the same is true for tables and it never drew my attention – it felt fine so far. So perhaps I'll get used to the image names too.

[jodator]

I'd rename them to insertImage (matches the new button name), resizeImage, uploadImage (matches the new button name) and checkTodoList.

:+1: sounds natural

I'd rename them to insertImage (matches the new button name), resizeImage, uploadImage (matches the new button name) and checkTodoList.

:+1: I'll second this, having those in tables seems OK so in the image would be OK also. Naming is tricky as we have both noun/verb features. Having two patterns is ok as bold is just OK and I don't know how we could use verb + noun there.

forwardDelete,

Reading above alongside others seems kinda odd, maybe deleteForward? But we could leave that as-is.

Side note: we do delete on backspace and forwardDelete on delete... at least on PC keyboard. Dunno how that's related to Mac.

insertParagraph,

insertTable.

~Those two also stands out a bit.~ So my assumption on reading was OK - it does insert paragraph as opposed to the paragraph command which will change block to a paragraph.

[Reinmar]

Reading above alongside others seems kinda odd, maybe deleteForward? But we could leave that as-is.

delete and forwardDelete are inherited from https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand. However, beforeInput fixes these names a bit so you have delete* and delete*Forward: https://www.w3.org/TR/input-events-2/#interface-InputEvent-Attributes. 

Actually, beforeInput is a good source as it's quite consistent in starting the action name from a verb. The only (funny) exception is historyUndo/Redo :D

Anyway, I'm fine to have deleteForward. This will read better and translate better to beforeInput.

[Reinmar]

Doh, one more question/problem – should class names be changed too? In general – should they follow the name under which the button/command is registered or the plugin name + sub-feature convention?

I'd go with following the name of the button/command.

[jodator]

I'd go with following the name of the button/command.

Oh yeah. Definitely :+1:.

[mlewand]

@Reinmar @jodator I wanted to sum up what needs to be done for @psmyrek. Please check the below and see if I'm missing something.

• Add verb+noun button names for all the noun+verb, while keeping old names for backward compatibility (it happens to be just 2 image buttons mentioned in "Noun + verb" of the main issue comment).
  
  
  
  • Be sure to update button class name too to follow this change.
  
  
• Rename Feature + action (verb) commands to verb+noun as mentioned in the comment.
  
  
  
  • Be sure to update command class name too to follow this change.
  
  
• Write an ADR.
• Document this (allowed naming formats) in the naming convention guide.
• Update changed references in the changelog.

[psmyrek]

@Reinmar @jodator What about the imageResize button (which fits in the "noun + verb" category)? Should it be also aligned with related command resizeImage?

[jodator]

Those should be resizeImage :+1: . But please share the summary so we can look at it once again.

[psmyrek]

Below is a summary of what I will change:

Buttons

imageInsert → insertImage
imageUpload → uploadImage
imageResize → resizeImage

Commands

imageInsert → insertImage
imageUpload → uploadImage
imageResize → resizeImage
todoListCheck → checkTodoList
forwardDelete → deleteForward

@Reinmar @jodator Should the commands also have an alias (old name) for backward compatibility, or just buttons?

[psmyrek]

Should the file names also be renamed to reflect the class name change (e.g. imageuploadcommand.js to uploadimagecommand.js)?

Currently, files are sorted nicely in the IDE (basing on the "feature" part), but some file names will not match the class name that will be defined in them:

src/
└── imageupload/
    ├── imageuploadcommand.js    ⭠ this file will contain the `UploadImageCommand` class
    ├── imageuploadediting.js
    ├── imageuploadprogress.js
    ├── imageuploadui.js
    └── utils.js

On the other hand, if we change the names of some files, then they will not sort well in the IDE anymore...

[jodator]

 Should the commands also have an alias (old name) for backward compatibility, or just buttons?

Both.

ps.: I've edited your post and added: forwardDelete → deleteForward change

Should the file names also be renamed to reflect the class name change (e.g. imageuploadcommand.js to uploadimagecommand.js)?

Yes. (remember to change also test file name and the describe entries as well.

For the file names vs feature path. I think that this is OK.

src/
└── imageupload/
    ├── uploadimagecommand.js    ⭠ this file will contain the `UploadImageCommand` class

Because the command name follows a specific convention here it will be easier to find UploadImageCommand class for editor.execute( 'uploadImage' ) command (as described in the docs.

This also brought my attention to keep in mind to update jsdocs paths as well when you'll rename a class. The UploadImageCommand will be in:

/**
 * @module image/imageupload/uploadimagecommand
 */