ckeditor5
ckeditor5 copied to clipboard
Make editor's application labels unique
📝 Provide detailed reproduction steps (if any)
- Open https://ckeditor.com/docs/ckeditor5/latest/features/lists/lists.html
- Open VoiceOver.
- Open rotor and switch to landmarks list.
✔️ Expected result
Every editor has a unique name on the list displayed by the rotor.
❌ Actual result
Every editor has the same name, "Rich Text Editor".
❓ Possible solution
Add the editor name after the static part of the label: "Rich Text Editor, <editor name>".
📃 Other details
- Browser: all (tested on Chrome)
- OS: all
- First affected CKEditor version: all
- Installed CKEditor plugins: N/A
The analogous issue is present in other screen readers.
If you'd like to see this fixed sooner, add a 👍 reaction to this post.
While working on this issue we can look how we solve this problem in CKE4.
We could add a config property that would allow for providing a human readable title / label. CKE4 has it for instance here: https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-applicationTitle
If no human-readable title is provided we should do the same thing we did in CKE4, so create a label like "editor1", "editor2", "editorn...".
Notes from the recent meeting:
- Let's use
editor.config.ui.name
for configuration. - For the time being, no properties such as
Editor#name
.- If a feature wants to know the name of the editor, it should use
editor.config.get( 'ui.name' )
instead. - To provide the incremental
EditorN
label for editors withouteditor.config.ui.name
config I think we should useeditor.config.define()
instead of generator proposed in the PR. We're doing this already.
- If a feature wants to know the name of the editor, it should use
- If there's just a single root (e.g. classic editor), we ditch its name in the label (
Editor editing area
instead ofEditor editing area: main
) because it only creates noise. - Multi root editors
-
editor.config.ui.name
should be used for the entire root constellation - As for naming individual roots, let's leave it as it is (
Rich Text Editor. Editing area: footerleft
) for now.
-
- Editor context should not pass
editor.config.ui.name
to the editor (we need to exclude it to avoid misleading duplication) - Comments editor
- All editors should be using the same name because the (a11y) context will be set on the parent of the thread in the future.
- Let's ignore
CKEditorError
integration because accessible name is not an instance name. - Let's ignore
CKEditorInspector
integration because accessible name is not an instance name
To provide the incremental
EditorN
label for editors withouteditor.config.ui.name
config I think we should useeditor.config.define()
instead of generator proposed in the PR. We're doing this already.
I just realized this won't work. See how the classic editor is initialized:
https://github.com/ckeditor/ckeditor5/blob/b0a183e5ddf131625d6c9c26fc575269e55890a1/packages/ckeditor5-editor-classic/src/classiceditor.js#L75-L90
- I think
EditorUI
(in this caseClassicEditorUI
) should useeditor.config.define( 'ui.name' )
because theEditor
class has nothing to do with the UI. - But... to allow
EditorUIView
(in this caseClassicEditorUIView
) propagate the value of the config in DOM, it needs to be known beforeEditorUI
gets created. - I don't remember why we decided to create a view first and then pass it to the UI. I don't see any elegant solution to work around this one.
Maybe I don't see something but... why not set it as one of options when creating ClassicEditorUIView
instance?
const view = new ClassicEditorUIView( this.locale, this.editing.view, {
shouldToolbarGroupWhenFull,
label: this.config.get( 'ui.name' )
} );
Or just pass whole config.
Or, cannot this be a dynamic value (I mean either bound to template or just observable)? It could be set afterwards. Why not?
label: this.config.get( 'ui.name' )
Because there must be a place that takes care of unnamed editors and increments "EditorN". Preferably by setting editor.config.define( 'ui.name', Editor${ ++i } )
. And the right place for this thing is definitely EditorUI
. And that does not exist yet at this point 😛
An update: After a F2F with PK we figured that we're gonna change the API for our editors' UIs ClassicEditorUI
, InlineEditorUI
etc. and implement ClassicEditorUI#setView( view: EditorUIView )
This will be a minor change because it's unlikely that any of these UIs is used as a base class by integrators and, at the same time, EditorUI#constructor( editor: Editor )
accepts editor instance only, so we're not changing the API of the base class.
Also, we figured we should probably use editor.config.ui.editorName
but let's figure it out during code review when we can see everything coming together.
To sum things up:
- Let's use
editor.config.ui.name
for configuration. - For the time being, no properties such as
Editor#name
.- If a feature wants to know the name of the editor, it should use
editor.config.get( 'ui.name' )
instead. - To provide the incremental
Rich Text Editor N
label for editors withouteditor.config.ui.name
config I think we should useeditor.config.define()
instead of generator proposed in the PR. We're doing this already.- To make it work, we should ditch the
view
argument in*EditorUI#constructor()
and use*EditorUI#setView( view: EditorUIView )
instead (minor BC) - I drafted a PoC and it seems to be working.
- To make it work, we should ditch the
- If a feature wants to know the name of the editor, it should use
- If there's just a single root (e.g. classic editor), we ditch its name in the label (
Editor editing area
instead ofEditor editing area: main
) because it only creates noise. - Multi root editors
-
editor.config.ui.name
should be used for the entire root constellation - As for naming individual roots, let's leave it as it is (
Rich Text Editor. Editing area: footerleft
) for now.
-
- Editor context should not pass
editor.config.ui.name
to the editor (we need to exclude it to avoid misleading duplication) - Comments editor
- All editors should be using the same name because the (a11y) context will be set on the parent of the thread in the future.
- Let's ignore
CKEditorError
integration because accessible name is not an instance name. - Let's ignore
CKEditorInspector
integration because accessible name is not an instance name
Does this have any impact on multi root editor implementations that people have? I mean changing how the view is set?
Does this have any impact on multi root editor implementations that people have? I mean changing how the view is set?
It should not because we're not changing the api of the base EditorUI
class.
Related - https://github.com/ckeditor/ckeditor5/issues/9731