Add input as focusable element inside panels #2457
Conversation
|
@msamsel Please add reference to the issue that is closed by this PR. |
plugins/panel/plugin.js
Outdated
| @@ -381,7 +381,7 @@ | |||
| // Move forward. | |||
| case 'next': | |||
| var index = this._.focusIndex, | |||
| links = this.element.getElementsByTag( 'a' ), | |||
| links = this._.getItems(), | |||
There was a problem hiding this comment.
These are no longer links. We should refer to this in a more generic way, like "focusables".
There was a problem hiding this comment.
This problem is repeated in couple other places.
plugins/panel/plugin.js
Outdated
| @@ -361,7 +361,7 @@ | |||
| * @returns {*|CKEDITOR.dom.nodeList} | |||
There was a problem hiding this comment.
This format is wrong. It will always return CKEDITOR.dom.nodeList, so please use this opportunity to adjust the API docs.
| @@ -0,0 +1,14 @@ | |||
| @bender-tags: 4.11.0, feature, 2453 | |||
| @bender-ui: collapsed | |||
| @bender-ckeditor-plugins: wysiwygarea,toolbar,panel | |||
There was a problem hiding this comment.
Please add space between the comma separated values just like you did for tags.
| * You can acess to `input` element with <kbd>TAB</kbd> key. | ||
|
|
||
| ### Unexpected: | ||
| * Input element is noto focusable with keyboard. |
| } | ||
| } ); | ||
| } | ||
| } ) |
| } ); | ||
| } | ||
| } ) | ||
| var editor = CKEDITOR.replace( 'editor', { |
There was a problem hiding this comment.
This variable is not used anywhere else. It is not needed.
| editor.ui.add( 'testplugin', CKEDITOR.UI_PANELBUTTON, { | ||
| label: 'test', | ||
| title: 'Test Panel', | ||
| modes: { wysiwyg: 1 }, |
There was a problem hiding this comment.
Non-empty literal objects should not be written in a single line.
CHANGES.md
Outdated
| @@ -12,6 +12,7 @@ New Features: | |||
| * [#706](https://github.com/ckeditor/ckeditor-dev/issues/706): Added different cursor style when selecting cells for [Table Selection](https://ckeditor.com/cke4/addon/tableselection) plugin. | |||
| * [#651](https://github.com/ckeditor/ckeditor-dev/issues/651): Text pasted using [Paste from Word](https://ckeditor.com/cke4/addon/pastefromword) preservers indentation in paragraphs. | |||
| * [#1176](https://github.com/ckeditor/ckeditor-dev/pull/1176): The [Balloon Panel](https://ckeditor.com/cke4/addon/balloonpanel) can be attached to selection instead of element. | |||
| * [#2453](https://github.com/ckeditor/ckeditor-dev/pull/2453): Add possibility to focus on `input` elements within [Panels](https://ckeditor.com/cke4/addon/panel). | |||
There was a problem hiding this comment.
This is marked as an API change, and should be placed in API changes section.
CHANGES.md
Outdated
| @@ -12,6 +12,7 @@ New Features: | |||
| * [#706](https://github.com/ckeditor/ckeditor-dev/issues/706): Added different cursor style when selecting cells for [Table Selection](https://ckeditor.com/cke4/addon/tableselection) plugin. | |||
| * [#651](https://github.com/ckeditor/ckeditor-dev/issues/651): Text pasted using [Paste from Word](https://ckeditor.com/cke4/addon/pastefromword) preservers indentation in paragraphs. | |||
| * [#1176](https://github.com/ckeditor/ckeditor-dev/pull/1176): The [Balloon Panel](https://ckeditor.com/cke4/addon/balloonpanel) can be attached to selection instead of element. | |||
| * [#2453](https://github.com/ckeditor/ckeditor-dev/pull/2453): Add possibility to focus on `input` elements within [Panels](https://ckeditor.com/cke4/addon/panel). | |||
There was a problem hiding this comment.
Wrong link. I can see the reason why it happened - you mad a copy'n'paste from the line above which introduced this issue. Always double check copied things.
| var links = this.element.getElementsByTag( 'a' ); | ||
| var item = links.getItem( this._.focusIndex = index ); | ||
| var focusables = this._.getItems(); | ||
| var item = focusables.getItem( this._.focusIndex = index ); |
There was a problem hiding this comment.
You could join these two subsequent var statements, as this is our code style since couple of years.
tests/plugins/panel/panel.js
Outdated
| @@ -23,6 +38,19 @@ | |||
| src = src.substring( 5, src.length - 1 ); | |||
|
|
|||
| assert.areSame( '', src, 'Frame source should be empty.' ); | |||
| }, | |||
|
|
|||
| // #2453 | |||
There was a problem hiding this comment.
More commonly we put issue references in parenthesis.
|
I have rebased the branch before merge. |
What is the purpose of this pull request?
New feature
This PR contains
What changes did you make?
Close #2453