Opened 7 years ago

Closed 7 years ago

#6082 closed New Feature (fixed)

Introduce the useComputedState setting to control default toolbar states

Reported by: Frederico Caldeira Knabben Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.4
Component: UI : Toolbar Version:
Keywords: IBM Cc: Satya Minnekanti, Damian

Description

There are two toolbar features that use computed values to indicate their on/off states: alignment and bidi.

In some cases though, it's better to give the user the possibility to explicitly apply these features. For example, a user may create contents in LTR, and publish them in various formats, including RTL.

To solve this case, we could introduce the useComputedState setting, which would be a generic flag indicating whether computed values or "real" values should be used by plugins on their state evaluation.

Attachments (4)

6082.patch (4.6 KB) - added by Frederico Caldeira Knabben 7 years ago.
6082_2.patch (5.6 KB) - added by Frederico Caldeira Knabben 7 years ago.
6082_3.patch (6.8 KB) - added by Frederico Caldeira Knabben 7 years ago.
6082_4.patch (7.0 KB) - added by Frederico Caldeira Knabben 7 years ago.

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by Frederico Caldeira Knabben

Attachment: 6082.patch added

comment:1 Changed 7 years ago by Frederico Caldeira Knabben

Owner: set to Frederico Caldeira Knabben
Status: newreview

comment:2 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_failed
  • We should also consider the "direction" CSS property in the bidi plugin (of course, it's priority is higher so it should be checked before the dir attribute).
  • We should also consider the "align" attribute in the justify plugin (of course, it's priority is lower so it should be checked after the "text-align" CSS property).
  • In the justify plugin, we should use .replace( alignRemoveRegex, '' ) even if we're not using the computed state.

Changed 7 years ago by Frederico Caldeira Knabben

Attachment: 6082_2.patch added

comment:3 Changed 7 years ago by Frederico Caldeira Knabben

Status: review_failedreview

comment:4 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_failed
  • Load the following html:
    <div style="text-align: right">
    	<p style="text-align: left">
    		This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    </div>
    
  • Click on the paragarph.
  • Click on the 'Left Justify' button.

The paragraph moves to the right instead of staying at the left.

Changed 7 years ago by Frederico Caldeira Knabben

Attachment: 6082_3.patch added

comment:5 in reply to:  4 Changed 7 years ago by Frederico Caldeira Knabben

Status: review_failedreview

Replying to Saare:

Note that in your tc, it *is* correct to have the paragraph moving right. The only problem is that it was not anymore possible to make it left aligned.

With this new patch, I've fixed that, and also optimized other cases, still trying to make the solution simple and efficient.

comment:6 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_failed
  • You can remove the alignRemoveRegex variable.
  • I can not anymore use the justify buttons AT ALL when `useComputedState' is true.

Changed 7 years ago by Frederico Caldeira Knabben

Attachment: 6082_4.patch added

comment:7 Changed 7 years ago by Frederico Caldeira Knabben

Status: review_failedreview

Ops... it was a small last minute change before creating the previous patch. One char changed, and everything should be ok now.

comment:8 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

Please add the @since tag to the config description before commiting.

comment:9 Changed 7 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: review_passedclosed

Fixed with [5784].

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