Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16881 closed feature (fixed)

Enable TablePlugins ModifyTable to use custom colour picker

Reported by: Nick Fenwick Owned by: bill
Priority: undecided Milestone: 1.9
Component: Editor Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

Not sure if this is an 'enhancement' or 'feature'. Several issues to address:

  1. dojox/editor/plugins/TablePlugins ModifyTable? plugin is currently hardwired to use dijit.ColorPalette? as its colour picker.
  2. ColorTableCell? is similarly hardwired to use dojox/widget/ColorPicker, via the template of CellColorDropdown?.
  3. The pickers are not passed any knowledge of their parent (the ModifyTable? plugin, in this case).
  4. TablePlugins? uses the old 1.0 API for registering plugins, which means they are only passed their command name and no other parameters from the args object used when constructing them.

I'm making modifications to support the assignment of a custom colour picker, and the ability to pass arbitrary properties to them (via their knowledge of their parent plugin, which with the newer _Plugin.registry API gets given all the properties used to construct it).

In our case this allows us to expose a <form> dom node to the colour picker so it may initialise its selectable colours based on some custom app logic, and these modifications enable generic abilities like this.

Attachments (2)

16881.patch (22.5 KB) - added by Nick Fenwick 7 years ago.
Patch for TablePlugins? and dojox/widget/ColorPicker, and tests file update.
16881.patch2 (5.8 KB) - added by Nick Fenwick 7 years ago.
New patch based on trunk after previous patch was applied.

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by Nick Fenwick

Attachment: 16881.patch added

Patch for TablePlugins? and dojox/widget/ColorPicker, and tests file update.

comment:1 Changed 7 years ago by Nick Fenwick

You may want to check the way I'm passing knowledge of the parent plugin to the colour picker. It's essential in our app that the picker can look in its parent for parameters. None of the dijit pickers do this, they just rely on having a value set on them to assign their initial colour.

Also note that because of the change to the new _Plugin.registry API, it seems the way the plugins are referenced must change. e.g. in the tests file, what used to be:

{name: 'dojox.editor.plugins.TablePlugins?', command: 'insertTable'}

Is now just:

{name: 'insertTable'},

(in fact perhaps that should be shortened to just 'insertTable')

Is this breaking change acceptable?

comment:2 Changed 7 years ago by bill

In [30912]:

Support colorPicker parameter to Editor's Table plugins, to specify custom color picker. Refs #16881 !strict. Patch from neekfenwick (CLA on file), thanks!

comment:3 Changed 7 years ago by bill

In [30913]:

Fix problem where clicking hue slider caused an onChange event, patch from neekfenwick (CLA on file), refs #16881 !strict.

Changed 7 years ago by Nick Fenwick

Attachment: 16881.patch2 added

New patch based on trunk after previous patch was applied.

comment:4 Changed 7 years ago by Nick Fenwick

Just attached a new patch. In actually using a custom picker in our proejct I ran into the problem that ColorTableCell? is hardwired to pop up a custom dijit, CellColorDropDown?, which owns a TooltipDialog? and contains Set,Cancel buttons and the picker to be used. Our picker has its own OK and Cancel buttons, so CellColorDropDown? is not a good dijit to use.

  1. Changed ColorTableCell? to accept a 'dropDown' argument which is can use as the drop down instead of CellColorDropDown?.
  2. Fixed some lifecycle problems that caused CellColorDropDown?'s startup to never be called, which meant its connect to dojox/widget/ColorPicker onChange doesn't fire and colour selection doesn't actually work.
  3. Fixed a silly bug where we never added w1 and w2 to the this.pickers array leading to exception on startup of ModifyTable?.
  4. Pass generic 'params' object in to dialogs and color pickers. This seems more friendly that passing 'parent' or 'parentPlugin', and exposes all the init data we might need.

These changes allow me to pop up our own frameless colour picker, initialise using information from the plugin params, and have its onChange events fire back correctly. It's beautiful, I tell ya.

These changes don't really become very apparent using the stock dijit pickers, but they're invaluable when using a custom picker with its own OK/Cancel buttons.

comment:5 Changed 7 years ago by bill

Owner: set to bill
Resolution: fixed
Status: newclosed

In [30933]:

remaining fixes for custom color pickers, thanks neekfenwick, fixes #16881 !strict

comment:6 Changed 7 years ago by bill

Milestone: tbd1.9
Note: See TracTickets for help on using tickets.