Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#10848 closed Bug (fixed)

Integrate remaining plugins with active filter

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

Description


Change History (12)

comment:1 Changed 5 years ago by Jakub Ś

Status: newconfirmed

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

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

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

Owner: Piotrek Koszuliński deleted
Status: assignedconfirmed

comment:4 Changed 5 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:5 Changed 5 years ago by Piotr Jasiun

TODO: When cursor is moved to nested editable (using editor#activeFilterChange):

  • disable not allowed buttons,
  • disable not allowed drop down elements (styles etc.),
  • remove or disable not allowed dialog fields (if it possible).
Last edited 5 years ago by Piotr Jasiun (previous) (diff)

comment:6 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

Changes:

  • hide not enabled styles,
  • disable style combo if none style is available,
  • disable font, color, language and indent buttons if they are not available,
  • disable not allowed (by requiredContent) fields and tabs in dialog,
  • disable tab if all of the fields are disabled,
  • add onStateUpdate method to richcombo and button plugins,
  • extend logic of style.checkApplicable method,
  • tests.

Changes in t/10848, and tests t/10848.

comment:7 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

core/style.js

  • Unfortunately this code is touching a method which has been changed on major. Better to rebase the branch before we review it finally.
  • Why half of documentation for "filter"? Better to at least make it clear that it is optional and a few words about it would not hurt.

plugins/language/plugin.js

  • On major we're now basing the language button state on the command state, so I think we don't need anything here any more.

As for the "onStateUpdate" idea, I would instead call it "refresh" to make it similar to commands (it is for the exact same need). Additionally, it is not need to pass "state" to this method. It can be parameter-less.

... and as usual some condign style issues :P

I was not able to do it a full review so far, but it is better to have the above worked first.

comment:8 Changed 5 years ago by Piotr Jasiun

Status: review_failedreview
  • both branches are now rebased on major,
  • merged checkApplicable method and tests for it,
  • I confirm that language plugin now is disabled without onStateUpdate so I removed this method,
  • renamed onStateUpdate( status) to refresh(),
  • fixed some code style.

comment:9 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

I’ve made a few commits for minor stuff. Still there are some issues:

  • The styles combo is behaving differently for block styles now. They don’t appear selected any more when active in the current selection.
  • On richcombo, updateState is now called on 'selectionChange’. What’s the case for this? This is kinda undesirable as we have already too much stuff on ‘selectionChange’.
  • On listblock, getItemElements has been added exclusively for testing purposes. This is not good as this code is useless for runtime and will endup in the release code. Better to move it to the tests code and access this._.items there directly.

comment:10 Changed 5 years ago by Piotr Jasiun

Status: review_failedreview
  • fixed block style issue,
  • do not call updateState on selectionChange (in richcombo) anymore, call refresh in format plugin instead,
  • moved getItemElements from dev to test.

comment:11 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:12 Changed 5 years ago by Piotr Jasiun

Resolution: fixed
Status: review_passedclosed

Closed.

Last edited 5 years ago by Piotr Jasiun (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy