Opened 7 years ago

Closed 7 years ago

#9481 closed New Feature (fixed)

Magicline: no accessibility support

Reported by: Frederico Caldeira Knabben Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.0.1
Component: Accessibility Version: 4.0
Keywords: IBM Cc: monahant@…, satya_minnekanti@…

Description

There must be an accessibility solution to make magicline usable through the keyboard only.

Change History (25)

comment:1 Changed 7 years ago by Frederico Caldeira Knabben

Status: newconfirmed

There is a proposal for this.

To define a keyboard shortcut. When this is invoked, use the current selection to determine whether the magicline feature should be available directly above the current block or not. If it should be available, then execute it. If the magicline UI should not be available, nothing should happen.

This sounds like the bet solution for this right now.

comment:2 Changed 7 years ago by Teresa Monahan

Cc: monahant@… added
Keywords: IBM added

comment:3 Changed 7 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.0.1

comment:4 Changed 7 years ago by Piotrek Koszuliński

Some details can be found in DUP of this tickets: #9695.

comment:5 Changed 7 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:6 Changed 7 years ago by Piotrek Koszuliński

Version: 4.0

comment:7 Changed 7 years ago by Olek Nowodziński

Status: assignedreview

Created branches:

  • t/9481@cksource
  • t/9481@ckeditor-tests-v4

Added accessibility support by assigning CTRL+< and CTRL+> keystrokes to accessSpaceBefore|After commands. Keystrokes are temporary; they'll most likely be changed to something else after the review.

Best testing goes on http://ckeditor4.t/ckeditor/plugins/magicline/samples/magicline.html

comment:8 Changed 7 years ago by Piotrek Koszuliński

Status: reviewreview_failed

Everything looks good except documentation and comments.

  1. There are outdated and incorrect doc strs for magicline_keystrokeBefore and magicline_keystrokeAfter - note that two of them concerns one setting.
  2. Comments for default keystrokes indicate CTRL+< and CTRL+> keystrokes when CTRL+, and CTRL+. are default ones.

BTW. CTRL+[ and CTRL+] may be better keystrokes, because they don't need SHIFT.

comment:9 Changed 7 years ago by Olek Nowodziński

Status: review_failedreview
  • Changed keystrokes to CTRL+[, CTRL+]
  • Enabled and fixed docs.
  • Renamed commands to avoid ambiguities.
  • Aligned dts to renamed commands.

comment:10 Changed 7 years ago by Olek Nowodziński

  • Updated magicline keystroke access to ignore accessible (like DIV, TABLE, etc.) elements when looking for sibling triggers to insert paragraph before/after them. For inaccessible elements (e.g. HRs), the feature works as before.
  • Updated tests to match current plugin implementation.

comment:11 Changed 7 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

Let's have CTRL+ALT+[ and CTRL+ALT+] as keystrokes, because the proposed ones conflict with browsers keystrokes on mac to navigate back and forth in the history.

comment:12 Changed 7 years ago by Olek Nowodziński

Status: review_failedreview

Pushed git:5c101ed, which updates default keystrokes and binds them to the plugin (forgot to do this before).

comment:13 Changed 7 years ago by Piotrek Koszuliński

Force pushed rebased branches.

comment:14 Changed 7 years ago by Piotrek Koszuliński

Status: reviewreview_passed

R+, but fix coding style issues in lines: 711, 821, 820 (var\t).

comment:15 Changed 7 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

comment:16 Changed 7 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

It's not possible to insert a new paragraph before the table with key board shortcut if first table cell has numbered/bulleted list as it's content. Same issue happens with inserting a paragraph after the table if last table cell has numbered/bulleted list as it's content.Can you please re-open this ticket and fix the issue

comment:17 Changed 7 years ago by Olek Nowodziński

Resolution: fixed
Status: closedreopened

comment:18 Changed 7 years ago by Satya Minnekanti

The keyboard shortcuts for magicline feature CTRL+ALT+[ and CTRL+ALT+] needs to be added to Accessibility Instructions dialog & it should clearly explain when to use CTRL+ALT+[ & when to use CTRL+ALT+]

comment:19 Changed 7 years ago by Satya Minnekanti

It's not possible to insert a new paragraph before the table with key board shortcut if first table cell has div as it's first element. Same issue happens with inserting a paragraph after the table if last table cell has div as it's last element.

comment:20 in reply to:  16 Changed 7 years ago by Olek Nowodziński

Replying to satya:

It's not possible to insert a new paragraph before the table with key board shortcut if first table cell has numbered/bulleted list as it's content. Same issue happens with inserting a paragraph after the table if last table cell has numbered/bulleted list as it's content.Can you please re-open this ticket and fix the issue

It's not possible to insert a new paragraph before the table with key board shortcut if first table cell has div as it's first element. Same issue happens with inserting a paragraph after the table if last table cell has div as it's last element.

The origin of this issue is known - there are also other similar cases that we must deal with (like definition list). It's also related to #9833.

Thanks for feedback!

comment:21 Changed 7 years ago by Olek Nowodziński

Status: reopenedreview

Pushed t/9481b branches to dev and tests, including:

  • Improved keystroke access so successive execution removes unchanged focus spaces. This change comes with rich undo/redo support and solves comment:16.
  • Smarter focus space search that solves related #9833.
  • Various tests for enhanced command/keystroke access.
  • Accessibility dialog docs for magicline keystroke access (comment:18).

comment:22 Changed 7 years ago by Piotrek Koszuliński

Owner: changed from Olek Nowodziński to Piotrek Koszuliński
Status: reviewassigned

Ok. There's an issue regarding last commit in that branch. Updating translations hasn't gone very well, because langtools got lost in that items array. I discussed that with Wiktor and I need to update them manually.

comment:23 Changed 7 years ago by Piotrek Koszuliński

Status: assignedreview

Pushed t/9481c with fixed translations.

comment:24 Changed 7 years ago by Piotrek Koszuliński

Status: reviewreview_failed
<div>div1</div>

<div>
  <p>p1</p>
  <ul><li>li1^</li></ul>
</div>

Accessing previous focus space doesn't work in this case. It works after switching "li1" into paragraph and from p1 paragraph.

Except this (if this is really an issue) commands seem to work perfectly.

PS. For one of us which will be closing this issue - there are tests on t/9481b.

comment:25 Changed 7 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_failedclosed

I extracted above issue to #9860. In fact it's a little bit different issue.

Fixed this issue with git:a6aa231.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy