Opened 4 years ago

Closed 4 years ago

#12865 closed Bug (fixed)

[Notification] Closing notification by clicking the close button should not blur the editor

Reported by: Piotrek Koszuliński Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version: 4.5.0 Beta
Keywords: Cc:

Description

  1. Open http://tests.ckeditor.dev:1030/tests/plugins/notification/manual/inline
  2. Create notification.
  3. Focus editor.
  4. Click "x".
  5. Editor is blurred.

Change History (9)

comment:1 Changed 4 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 4 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:3 Changed 4 years ago by Piotr Jasiun

Status: assignedreview

Changes in t/12865.

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

Status: reviewreview_failed

Manual test is needed (tags: 4.5.0, tc).

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

I'm also unsure whether the patch is correct. What if editor wasn't focused before the hide() method was called? Perhaps we can ignore this and perhaps it's even better because the focus stays in the editor, but that proves that we need a manual test. I'm especially curious if focusing will not cause scroll, so the tc could contain a long inline editor.

comment:6 in reply to:  5 Changed 4 years ago by Piotr Jasiun

Status: review_failedreview

Replying to Reinmar:

I'm also unsure whether the patch is correct. What if editor wasn't focused before the hide() method was called? Perhaps we can ignore this and perhaps it's even better because the focus stays in the editor, but that proves that we need a manual test. I'm especially curious if focusing will not cause scroll, so the tc could contain a long inline editor.

I focus editor on hide only if notification is handled by the notification area, so the notification is above the editors content. It does not look like unexpected behavior than editor get focus if I click on the element above the content. Focus does not cause scroll (at least on Chrome), but no strange would it be if the editor move user to the position of the selection when he close notification.

I rebased branch on the newest major and pushed manual test. Changes in t/12865.

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

Status: reviewreview_failed

Guess what happens when you're e.g. uploading an image in one editor and working in another one? :>

comment:8 Changed 4 years ago by Piotr Jasiun

Status: review_failedreview

I moved the focus call from the hide method to the click listener. It makes more sense and works fine. Branch rebased on the newest major. Changes in the same branch.

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

Resolution: fixed
Status: reviewclosed

Fixed on major with git:f887b79.

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