Opened 4 years ago

Closed 4 years ago

#12867 closed Bug (fixed)

[Notification] Notification area should not have padding

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. See that you if you click next (<20px) to the notification the editor loses focus.

Change History (6)

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

I pushed changes t/12867 so now you can click next to or below the notification. I apply only top margin, read it and use as a left, right and bottom distance from the editors border when the notification is next to it.

This solution has 2 drawbacks:

  • because notification still has top margin you will blur the editor if you click between notifications of between notification and a toolbar,
  • you can not set different margin for the top/bottom/left/right, always top margin is used; also this margins works somehow magically (the top margin is used for all 4 directories) what may be confusing if someone works on the custom skin.

For the full solution, margin could be read on loading, removed and the position set by JS, but it would be too time consuming to implement and test properly and I believe that the current solution is good enough.

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

Status: reviewassigned

Hmm... I haven't thought that these margins may be important. In this case I've got a different proposal, but I'm not sure if it's going to work.

For the area add pointer-events: none and then for each notification add pointer-events: auto. I see that on Chrome it works perfectly, but other browsers may not allow to reset the pointer-events to auto once it's none. (PS. on Chrome I checked initial, but AFAICS auto is the default. You can try some other values if auto does not work).

comment:5 Changed 4 years ago by Piotr Jasiun

Status: assignedreview

pointer-events combination seems to works fine on Chrome, Firefox and IE (tested with 11 and 8), so it seems to be a perfect solution. Great idea! Changes in t/12867b.

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

Resolution: fixed
Status: reviewclosed

Fixed on major with git:56e551d.

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