Opened 5 years ago

Closed 4 years ago

#10916 closed Bug (fixed)

Magicline icon is not mirrored in RTL environments

Reported by: Teresa Monahan Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.4.4
Component: General Version: 4.0
Keywords: IBM Cc: Satya Minnekanti, Irina

Description

To Reproduce:

  • Open the uiLanguages sample and select an RTL language from the dropdown e.g. Arabic
  • Insert a table at the start of the editor
  • Hover above the editor so that the magicline UI is displayed.

Problem: The icon is not mirrored for RTL languages.

The following code is from magicline/plugin.js. As you can see there is no support for loading a different icon for rtl/ltr languages:

iconPath = this.path + 'images/' + ( env.hidpi ? 'hidpi/' : '' ) + 'icon.png';

Change History (8)

comment:1 Changed 5 years ago by Jakub Ś

Status: newconfirmed
Version: 4.2.2 (GitHub - master)4.0

I confirm this - icon shows on left side (correct) but it isn't mirrored.

comment:2 Changed 4 years ago by Christophe Guillou

Hi, just checking if that's something we can get fixed for the next release? To what I can see in the source, the fix is very straightforward, we would just need to add some more logic in

L571:

iconPath = this.path + 'images/' + ( env.hidpi ? 'hidpi/' : ) + 'icon.png';

to select the 'mirrored icon' based on the rtl setting (using: that.rtl)

Can you please give us an update if that's realistic get this incorporated for the next milestone?

comment:3 Changed 4 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedreview

Pushed fix to branch:t/10916.

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

Status: reviewreview_failed

On IE9 there's a black background created by the filter. The arrow has round borders so the background is visible there.

comment:5 in reply to:  4 Changed 4 years ago by Olek Nowodziński

Status: review_failedreview

Replying to Reinmar:

On IE9 there's a black background created by the filter. The arrow has round borders so the background is visible there.

Yeah, I know about it but I decided that it's OK considering the simplicity of solution and small market share.

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

Milestone: CKEditor 4.4.4
Status: reviewreview_failed

I see no problems with the second icon file and if we are doing anything, let's do it right.

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

Status: review_failedreview

comment:8 Changed 4 years ago by Marek Lewandowski

Resolution: fixed
Status: reviewclosed

Good job, fixed with git:67433b0695 (merged to master) at dev.

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