carbon-design-system / carbon-charts

:bar_chart: :chart_with_upwards_trend:⠀Robust dataviz framework implemented using D3 & typescript
https://charts.carbondesignsystem.com
Apache License 2.0
915 stars 184 forks source link

[Bug]: Demo / Radar Chart / Missing Datapoints - logs errors in JavaScript console #1555

Open nstuyvesant opened 1 year ago

nstuyvesant commented 1 year ago

Name

Nate Stuyvesant

Are you an IBM employee?

What happened?

If you navigate to any of the demos in these sections of the demo site for core, Angular, React, Svelte, or Vue UTILITY / Highlights UTILITY / Toolbar UTILITY / Zoom bar

you will get this JavaScript console error...

[Error] Error: Invalid negative value for <rect> attribute width="-1"

    (anonymous function) (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:255825)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:222512)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    brush (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:222392)
    call (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:265624)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:557034)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530224)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530134)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530098)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530224)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530134)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530098)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:607802)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:607401)
    dispatchEvent
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:241198)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:239995)
    invokeFunc (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:1197145)

If you navigate to SIMPLE CHARTS / Meter or Meter (proportional) (any example), you will get this error...

[Error] Error: Invalid value for <circle> attribute cy="calc(1em / 2)"

    (anonymous function) (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:255825)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:314022)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:310860)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530224)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530134)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530098)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530224)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530134)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530098)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530224)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530134)
    each (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:266137)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:530098)
    forEach
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:607802)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:607401)
    dispatchEvent
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:241198)
    (anonymous function) (main.fbca26b7bc5e95861b32.bundle.js:1:239995)
    invokeFunc (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:1197145)

If you go to SIMPLE CHARTS / Radar / Radar - Missing datapoints, you will get this error...

[Error] Error: Problem parsing d="M0,-59.15L118.581,-38.529L29.953,41.226L-25.138,34.599LNaN,NaNZ"

    (anonymous function) (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:208226)
    tick (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:201618)
    (anonymous function) (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:201156)
    (anonymous function) (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:200325)
    timerFlush (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:199092)
    wake (vendors~main.fbca26b7bc5e95861b32.bundle.js:2:199127)

These happen on Safari 16.4 and Chrome 112.

Version

@carbon/charts@1.7.5 but I suspect these go back further in time since the charts still render.

Data & options used

Just using demo site

Relevant log output

See above.

Codesandbox example

https://charts.carbondesignsystem.com/?path=/story/utility-highlights--bounded-area-time-series-natural-curve

What priority level would this be in your opinion?

P0

nstuyvesant commented 1 year ago

@theiliad - noticed this when I was running locally. I was planning to submit a PR with fixes for lint errors in core but I didn't want to do that before you got a chance to see that these problems are not related to what I am working on.

nstuyvesant commented 1 year ago

The first problem (UTILITY/Highlights) is with src/components/axes/grid-brush.ts:

If the width constant <1, the error will occur...

            const brush = brushX()
                .extent([
                    [0, 0],
                    [width - 1, height]
                ])
                .on('start brush end', brushEventHandler)
                .on('end.brushed', brushed)

This solves that particular issue as long as brush is defined as a variable above instead of as a constant.

            if (height != 0 && width != 0) {
                // leave some space to display selection strokes beside axes
                brush = brushX()
                    .extent([
                        [0, 0],
                        [ width - 1, height]
                    ])
                    .on('start brush end', brushEventHandler)
                    .on('end.brushed', brushed)

                brushArea.call(brush)
            }

Adding this conditional to src/components/axes/chart-clip.ts fixes several of the other ones...

        if (xScaleEnd - xScaleStart > 0) {
            clipRect
            .attr('x', xScaleStart)
            .attr('y', yScaleStart)
            .attr('width', xScaleEnd - xScaleStart)
            .attr('height', yScaleEnd - yScaleStart)
        }
nstuyvesant commented 1 year ago

Only one I need to research is the SIMPLE CHARTS/Radar/Radar - Missing data points having the NaN in the generated path.

nstuyvesant commented 1 year ago

I may just submit the fixes with my big PR

nstuyvesant commented 1 year ago

The radar chart is erroring on src/components/graphs/radar.ts on the transition() call here...

                enter
                    .append('path')
                    .attr('opacity', 0)
                    .attr('transform', `translate(${c.x}, ${c.y})`)
                    .attr('fill', 'none')
                    .call((selection: any) => 
                        selection
                            .transition() // BUG: 19 console error for charts with missing datapoints (generated path has NaN in it)
                            .call((t: any) =>
                                this.services.transitions.setupTransition({
                                    transition: t,
                                    name: 'radar_y_axes_enter',
                                    animate
                                })
                            )
                            .attr('opacity', 1)
                            .attr('d', (tick: any) => radialLineGenerator(shapeData(tick)))
                    )
nstuyvesant commented 1 year ago

In the demo, SIMPLE CHARTS / Meter (proportional) / Proportional Meter Chart - peak and statuses, error is logged "Error: attribute x: Expected length, NaN" because of this line in src/components/essentials/title-meter.ts:

.attr('x',
  this.model.getStatus() ? containerWidth - meter.total.paddingRight : containerWidth
)

In this case...

// this.model.getStatus() == 'warning'
// containerWidth == '100%'
// meter.total.paddingRight == 24

// The problem is you can't subtract '100%' - 24
theiliad commented 1 year ago

I may just submit the fixes with my big PR

@nstuyvesant let's pls put these in a separate PR 🙂 That PR is already super large, which is understandable