Update dependency @elastic/charts to v23.0.0 by nickofthyme · Pull Request #79226 · elastic/kibana

nickofthyme · 2020-10-01T21:56:12Z

Summary

This PR contains the following updates:

Package	Type	Update	Change
@elastic/charts	dependencies	major	`21.1.2` -> `23.0.0`

Fixes accidental breaking change in v21.1.1 see elastic/elastic-charts#830

Note on BREAKING CHANGES for Reviewers

There is just a main breaking change from the previous version and you can encounter it only if you use the headerFormatter to change the formatting of the tooltip header.
The breaking change is the following: the TooltipValue.value is now raw value (not formatted with the X axis configured tick formatter) and TooltipValue.formattedValue is now the actual formatted value.

Release Notes

elastic/elastic-charts

23.0.0 (2020-09-30)

Bug Fixes

render continuous line/area between non-adjacent points (#833) (9f9892b), closes #825

Features

debug state flag added to chart status (#834) (83919ff)
expose datum as part of GeometryValue (#822) (dcd7077)

BREAKING CHANGES

when rendering non-stacked line/area charts with a continuous x scale and no fit function,
the line/area between non-consecutive data points will be rendered as a continuous line/area without adding an uncertain dashed line/ semi-transparent area that connects the two, non-adjacent, points.

22.0.0 (2020-09-22)

Bug Fixes

breaking change in patch release of 21.1.1 (d0ddc45), closes #830
legend dark mode hover color (#820) (5227b2e)
line path with ordered xValues (#824) (5a73a3a)
axis: style overrides not applied to axis dimensions (#829) (62172c4)
remove unused redux dev middlewares (#812) (b2679e7)
build issues and tooltip formatting issues (#810) (74d9ae0)

Features

cancel brush/click event with escape key (#819) (b599d13)
show crosshair for external pointer events (#817) (f591a6a)
blind sorting option for vislib (#813) (8afce43)
order ordinal values by sum (#814) (5b2758b)
series: add simple mark formatter (#775) (ab95284)

BREAKING CHANGES

caused by changes in add an option to sort the stat panel results by label or value #810 see when using topN is it possible to pass values of fields #830 for more info
TooltipValue.value is now raw value and TooltipValue.formattedValue is now the
string formatted value.

@elastic/charts update procedure

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

spalger

LGTM

nickofthyme · 2020-10-01T23:52:01Z

The type errors seem unrealated 🤔

ERROR x-pack failed
      x-pack/plugins/infra/public/containers/ml/modules/metrics_hosts/module_descriptor.ts:81:36 - error TS2339: Property 'partition_field_name' does not exist on type '{ detector_description: string; function: string; field_name: string; }'.

      81       analysis_config.detectors[0].partition_field_name = partitionField;
                                            ~~~~~~~~~~~~~~~~~~~~

      x-pack/plugins/infra/public/containers/ml/modules/metrics_hosts/module_descriptor.ts:118:36 - error TS2339: Property 'aggregations' does not exist on type '{ job_id: string; indices: string[]; indices_options: { allow_no_indices: boolean; }; query: { bool: { must: { exists: { field: string; }; }[]; }; }; } | { job_id: string; indices: string[]; indices_options: { ...; }; query: { ...; }; chunking_config: { ...; }; aggregations: { ...; }; } | { ...; }'.
        Property 'aggregations' does not exist on type '{ job_id: string; indices: string[]; indices_options: { allow_no_indices: boolean; }; query: { bool: { must: { exists: { field: string; }; }[]; }; }; }'.

      118           ...defaultDatafeedConfig.aggregations,
                                             ~~~~~~~~~~~~

      x-pack/plugins/infra/public/containers/ml/modules/metrics_k8s/module_descriptor.ts:113:29 - error TS2339: Property 'aggregations' does not exist on type '{ job_id: string; indices: string[]; indices_options: { allow_no_indices: boolean; }; query: { bool: { must: { exists: { field: string; }; }[]; }; }; } | { job_id: string; indices: string[]; indices_options: { ...; }; query: { ...; }; chunking_config: { ...; }; aggregations: { ...; }; } | { ...; }'.
        Property 'aggregations' does not exist on type '{ job_id: string; indices: string[]; indices_options: { allow_no_indices: boolean; }; query: { bool: { must: { exists: { field: string; }; }[]; }; }; }'.

      113       defaultDatafeedConfig.aggregations[DEFAULT_K8S_PARTITION_FIELD].aggregations;
                                      ~~~~~~~~~~~~


      Found 3 errors.

markov00 · 2020-10-02T07:31:52Z

@elasticmachine merge upstream

kibanamachine · 2020-10-02T09:06:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7e2845f

Metrics [docs]

@kbn/ui-shared-deps asset size

id	before	after	diff
`kbn-ui-shared-deps.@elastic.js`	2.9MB	2.9MB	+8.3KB
`kbn-ui-shared-deps.js`	5.2MB	5.2MB	+8.0B
total			+8.3KB

distributable file count

id	before	after	diff
`default`	45825	45832	+7
`oss`	26530	26537	+7

History

💔 Build #78974 failed eba480f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

markov00 · 2020-10-02T09:18:15Z

@shahzad31 I'd like to have your review on the tooltip header used in the following file:

kibana/x-pack/plugins/apm/public/components/app/RumDashboard/Charts/PageLoadDistChart.tsx

Lines 75 to 83 in 07ebb81

    
           const headerFormatter: TooltipValueFormatter = (tooltip: TooltipValue) => { 
        
             return ( 
        
               <div> 
        
                 <p> 
        
                   {tooltip.value} {I18LABELS.seconds} 
        
                 </p> 
        
               </div> 
        
             ); 
        
           };

We changed the current format of the TooltipValue with:

TooltipValue.value is now raw value and TooltipValue.formattedValue is now the
string formatted value.
could you please check if that alter your current chart?

Also note that this update fixes the issues you have reported here: elastic/elastic-charts#825

markov00 · 2020-10-02T09:25:29Z

@flash1293 I want to point your attention to this code here, can you please check if that upgrade doesn't affect that?

kibana/x-pack/plugins/lens/public/xy_visualization/expression.tsx

Lines 388 to 390 in 198c5d9

    
           tooltip={{ 
        
             headerFormatter: (d) => safeXAccessorLabelRenderer(d.value), 
        
           }}

markov00 · 2020-10-02T09:27:15Z

@peteharverson could you please review this dependency upgrade and in particular if the mentioned breaking changes affect this part of your code:

kibana/x-pack/plugins/ml/public/application/datavisualizer/index_based/components/field_data_card/metric_distribution_chart/metric_distribution_chart.tsx

Lines 59 to 72 in bf04235

    
           const headerFormatter: TooltipValueFormatter = (tooltipData: ChartTooltipValue) => { 
        
             const xValue = tooltipData.value; 
        
             const chartPoint: MetricDistributionChartData | undefined = chartData.find( 
        
               (data) => data.x === xValue 
        
             ); 
        
             return ( 
        
               <MetricDistributionChartTooltipHeader 
        
                 chartPoint={chartPoint} 
        
                 maxWidth={width / 2} 
        
                 fieldFormat={fieldFormat} 
        
               /> 
        
             ); 
        
           };

dej611 · 2020-10-02T10:13:04Z

@markov00 on the Lens side it's ok that line as we convert the particular case to string before to give it to the chart. The value and formattedValue is the same content for now. No breaking changes. (Tested locally already)

shahzad31 · 2020-10-02T11:11:09Z

@markov00 fix for elastic/elastic-charts#825 seems to be working great

shahzad31 · 2020-10-02T11:12:26Z

Though tooltip formatting is gone

markov00 · 2020-10-02T13:22:30Z

@shahzad31 probably this is the right image (your seems not being uploaded completely)

I think the issue here is that you are using the labelFormat in the Y Axis component and using the tickFormat only on the Overall LineSeries. The other lines are not receiving the formatted because of that.
The possible way to fix this is:

switch the Y Axis formatter to the tickFormat prop instead of the labelFormat
add a tickFormat for every BreakdownSeries series

 <Axis
            id="left"
            title={I18LABELS.percPageLoaded}
            position={Position.Left}
            labelFormat={(d) => d + ' %'} <---- this formatter is used only for the axis, not for the series, use tickFormat instead
          />
          <LineSeries
            sortIndex={0}
            fit={Fit.Linear}
            id={'PagesPercentage'}
            name={I18LABELS.overall}
            xScaleType={ScaleType.Linear}
            yScaleType={ScaleType.Linear}
            data={data?.pageLoadDistribution ?? []}
            curve={CurveType.CURVE_CATMULL_ROM}
            lineSeriesStyle={{
              point: { visible: false },
              line: { strokeWidth: 3 },
            }}
            color={euiChartTheme.theme.colors?.vizColors?.[1]}
            tickFormat={(d) => numeral(d).format('0.0') + ' %'} <--- this is series specific
          />
          {breakdown && (
            <BreakdownSeries.  <---- this component, internally doesn't have any formatter
              key={`${breakdown.type}-${breakdown.name}`}
              field={breakdown.type}
              value={breakdown.name}
              percentileRange={percentileRange}
              onLoadingChange={(bLoading) => {
                setBreakdownLoading(bLoading);
              }}
            />
          )}

Update
looking at the screenshot, the best approach is to add the tickFormat to the BreakdownSeries as you are using a different formatted for the axis and for the series (you are limiting the number of decimal points only on the series data)

This is not a breaking change introduced with this PR, but it was already there since version 21.1.0

darnautov · 2020-10-02T16:52:45Z

Tested and LGTM

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

upgrade @elastic/charts to v23.0.0

eba480f

nickofthyme requested review from a team, flash1293, markov00 and shahzad31 October 1, 2020 21:56

nickofthyme requested a review from a team as a code owner October 1, 2020 21:56

nickofthyme added dependencies Pull requests that update a dependency file release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Oct 1, 2020

spalger approved these changes Oct 1, 2020

View reviewed changes

Merge branch 'master' into upgrade-elastic-charts

7e2845f

markov00 requested a review from peteharverson October 2, 2020 09:27

shahzad31 approved these changes Oct 2, 2020

View reviewed changes

markov00 approved these changes Oct 2, 2020

View reviewed changes

dej611 approved these changes Oct 2, 2020

View reviewed changes

markov00 requested a review from darnautov October 2, 2020 17:18

markov00 merged commit e9fd390 into elastic:master Oct 2, 2020

markov00 mentioned this pull request Oct 2, 2020

[7.x] upgrade @elastic/charts to v23.0.0 (#79226) #79331

Merged

nickofthyme deleted the upgrade-elastic-charts branch October 2, 2020 20:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependency @elastic/charts to v23.0.0#79226

Update dependency @elastic/charts to v23.0.0#79226
markov00 merged 2 commits intoelastic:masterfrom
nickofthyme:upgrade-elastic-charts

nickofthyme commented Oct 1, 2020 •

edited by markov00

Loading

Uh oh!

spalger left a comment

Uh oh!

nickofthyme commented Oct 1, 2020 •

edited

Loading

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

kibanamachine commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

dej611 commented Oct 2, 2020 •

edited

Loading

Uh oh!

shahzad31 commented Oct 2, 2020

Uh oh!

shahzad31 commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020 •

edited

Loading

Uh oh!

darnautov commented Oct 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

nickofthyme commented Oct 1, 2020 • edited by markov00 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Note on BREAKING CHANGES for Reviewers

Release Notes

23.0.0 (2020-09-30)

Bug Fixes

Features

BREAKING CHANGES

22.0.0 (2020-09-22)

Bug Fixes

Features

BREAKING CHANGES

Checklist

Uh oh!

spalger left a comment

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

kibanamachine commented Oct 2, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/ui-shared-deps asset size

distributable file count

History

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020

Uh oh!

dej611 commented Oct 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shahzad31 commented Oct 2, 2020

Uh oh!

shahzad31 commented Oct 2, 2020

Uh oh!

markov00 commented Oct 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darnautov commented Oct 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nickofthyme commented Oct 1, 2020 •

edited by markov00

Loading

nickofthyme commented Oct 1, 2020 •

edited

Loading

dej611 commented Oct 2, 2020 •

edited

Loading

markov00 commented Oct 2, 2020 •

edited

Loading