Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13852 closed defect (fixed)

ColorPalette: performance issues

Reported by: bill Owned by: bill
Priority: high Milestone: 1.7
Component: Dijit Version: 1.6.1
Keywords: Cc: Ed Chatelain
Blocked By: Blocking:

Description (last modified by bill)

ColorPalette and thus Editor (with the foreColor and backgroundColor plugins) is slow to create. It's gotten worse over releases. Numbers from Ed's benchmarks for creating a 7x10 ColorPalette:

  • 1.5.1: 332ms
  • 1.6.1: 521ms
  • 1.7: 634 - 746ms

Looks like the main issue is all the connections per cell (ondijitclick, mouseenter, mouseleave, focus, etc.).

The other issue is that the drop down ColorPalette shouldn't be created until the user presses the toolbar button. IIRC this is how I did ScrollingTabController's menu button. Tracking that in #13866.

Attachments (1)

colorPalette_no_hover_pseudoclass.patch (3.1 KB) - added by bill 7 years ago.
Now that _CssStateMixin uses event delegation, could restore dijitPaletteCellHover class application to hovered node without a performance hit. Refs #13852, #14568 !strict.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 8 years ago by bill

Note: originally checked in [26469] referencing this ticket but now associating with #13866.

Last edited 8 years ago by bill (previous) (diff)

comment:2 Changed 8 years ago by bill

Resolution: fixed
Status: newclosed

Typed the wrong id in the commit message, it should have gone here:

(In [26471]) Speed up ColorPalette creation time:

(1) Put ondijitclick handler on the TABLE node rather than each TD.

(2) Use :hover and :active CSS pseudo-selectors rather than setting dijitPaletteCellHover / dijitPaletteCellActive. Needed to increase selectivity of dijitPaletteCellSelected selector so it wins over dijitPaletteCell:hover selector. (Hovering the selected cell should have no effect, since clicking that cell has no effect.) Note that hover state is no longer shown on IE6, but works on IE7+. Could add code to make IE work (monitoring mouseout, mouseover, mousedown, mouseup on this.gridNode) but I don't think it's worth the code bloat.

Fixes #1852 !strict.

comment:3 Changed 8 years ago by bill

Description: modified (diff)

Changed 7 years ago by bill

Now that _CssStateMixin uses event delegation, could restore dijitPaletteCellHover class application to hovered node without a performance hit. Refs #13852, #14568 !strict.

Note: See TracTickets for help on using tickets.