Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#12290 closed defect (fixed)

[patch][ccla] Charting Widget without fill/stroke specified will not use the theme fill/stroke for chart backgroud

Reported by: cjolif Owned by: Chris Mitchell
Priority: high Milestone: 1.7
Component: Charting Version: 1.6.0b1
Keywords: 1.7-mobile Cc: Chris Mitchell
Blocked By: Blocking:

Description

See the attached test case, if you do not specify fill/stroke on Chart widget it does not use the theme chart fill instead. If you use the Chart core component it works.

The candidate patch is making sure the behavior is consistent against the widget and the core component.

That is:

  • no value is specified the theme is used
  • null is specified nothing is drawn
  • a color is specified, it is used to draw

Attachments (2)

test_fillstroke.html (4.3 KB) - added by cjolif 9 years ago.
Chart.patch (594 bytes) - added by cjolif 9 years ago.

Download all attachments as: .zip

Change History (18)

Changed 9 years ago by cjolif

Attachment: test_fillstroke.html added

comment:1 Changed 9 years ago by cjolif

Component: General is wrong, this is of course a charting issue. Can't find a way to edit this.

comment:2 Changed 9 years ago by Adam Peller

Component: GeneralCharting
Owner: changed from anonymous to Eugene Lazutkin

There appears to be a typo in the patch (a stray 'f') Also, I think there's special syntax to declare variables for the parser but not actually include them in the code, something like /*===== =====*/

comment:3 Changed 9 years ago by Adam Peller

is there something missing from the patch?

comment:4 Changed 9 years ago by cjolif

I have fixed the typo. For the special syntax, I will try to find about it and come back to you.

Changed 9 years ago by cjolif

Attachment: Chart.patch added

comment:5 Changed 9 years ago by cjolif

Can't find anything about the /*===== =====*/ syntax.

@peller is there something missing from the patch?

Are your referring to something in particular? From my point of view nothing is missing except if we want to use the above syntax.

comment:6 Changed 9 years ago by Adam Peller

oh, it's as simple as not defining those vars to null. Got it.

I don't think the special syntax is documented. Anyway, Eugene ought to be able to take it from here. Thanks, Christophe!

comment:7 Changed 9 years ago by cjolif

Yes, that's exactly just defining the variables was causing trouble because the charting code is testing against undefined later down the road. Thanks noticing the typo!

comment:8 Changed 9 years ago by bill

Keywords: 1.7-mobile added

comment:9 Changed 9 years ago by Chris Mitchell

Milestone: tbd1.7

comment:10 Changed 9 years ago by cjolif

Not that this patch also fix clipping issues with charting widget. Indeed the clipping is done by the chart fill and without this patch the chart fill of the theme will be overridden by the null value of the widget preventing clipping to happen.

comment:11 Changed 9 years ago by Chris Mitchell

Owner: changed from Eugene Lazutkin to Chris Mitchell
Status: newassigned

comment:12 Changed 9 years ago by Chris Mitchell

Resolution: fixed
Status: assignedclosed

comment:13 Changed 9 years ago by Chris Mitchell

Resolution: fixed
Status: closedreopened

comment:14 Changed 9 years ago by Chris Mitchell

Resolution: fixed
Status: reopenedclosed

comment:15 Changed 9 years ago by Eugene Lazutkin

Again: all is fine and dandy, but no change set.

comment:16 Changed 9 years ago by bill

Cc: Chris Mitchell added

[24420] and [24430] (from Chris again)

Note: See TracTickets for help on using tickets.