-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update dependency @elastic/charts to v37 (master) #113968
Update dependency @elastic/charts to v37 (master) #113968
Conversation
const allPieSlicesColor = await pieChart.getAllPieSliceColor('80,000'); | ||
const whitePieSliceCounts = allPieSlicesColor.reduce((count, color) => { | ||
// converting the color to a common format, testing the color, not the string format | ||
return chroma(color).hex().toUpperCase() === '#FFFFFF' ? count + 1 : count; | ||
}, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've relaxed this check to verify that the color on the slice is white
removing the check of the actual color string format.
getAllPieSliceStyles
was also renamed to getAllPieSliceColor
because we always return a set of colors, not styles
@elasticmachine merge upstream |
Pinging @elastic/apm-ui (Team:apm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uptime Changes LGTM !!
@@ -91,7 +92,7 @@ export const DocumentCountChart: FC<Props> = ({ | |||
[data] | |||
); | |||
|
|||
const onBrushEnd: BrushEndListener = ({ x }) => { | |||
const onBrushEnd = ({ x }: XYBrushEvent) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove BrushEndListener
here if it is force casted below then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment we have two ways of doing that:
- force casting it as
BrushEndListener
- force casting the arguments of the BrushEndListener to XYBrushEvent
Both methods are not ideal at the moment. We will introduce soon a better type system for those union types (probably through a property to discriminate between union types)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand that. I was asking as in this component both routes have been chosen: force casting the arguments and few lines below force casting the function.
My point was: can we leave the force cast in one place rather than duplicate?
Not a big blocker, just curious on the reasons of the move.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ML changes LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data Discovery team owned files LGTM, just type changes. Didn't test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
watcher changes lgtm, didn't test locally
Something flaky is going on with the last CI run |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APM changes look good
@elasticmachine merge upstream unrelated issues in CI |
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / general / X-Pack Accessibility Tests.x-pack/test/accessibility/apps/index_lifecycle_management·ts.Index Lifecycle Management "before all" hook for "Add policy to index template modal"Standard Out
Stack Trace
Kibana Pipeline / general / X-Pack Accessibility Tests.x-pack/test/accessibility/apps/index_lifecycle_management·ts.Index Lifecycle Management "after all" hook for "Index templates flyout"Standard Out
Stack Trace
Kibana Pipeline / general / X-Pack Accessibility Tests.x-pack/test/accessibility/apps/index_lifecycle_management·ts.Index Lifecycle Management "before all" hook for "Add policy to index template modal"Standard Out
Stack Trace
and 2 more failures, only showing the first 3. Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline about the typings thing and sorted out.
Tested locally and all good. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presentation team changes LGTM! Anything that makes the dashboard_state
functional test better is a win in my books, thanks for doing this @markov00
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Co-authored-by: Marco Vettorello <[email protected]>
[skip-ci]
This PR contains the following updates:
34.2.1
->37.0.0
The code changes are due to the following
@elastic/charts
breaking changes:the
textContrast
textInvertible
andtextOpacity
in theFont
type has been removed. This affect theHeatmapConfig
type in thecell.label
,xAxisLabel
, andyAxisLabel
properties. This also affect thePartitionConfig
properties:fillLabel
andlinkLabel
. ThetextInvertible
andtextContrast
properties are also removed from theDisplayValueStyle
used to display bar values labels. The mentioned changes are replaced with the following behavior: always invert the text color depending on the color contrast with its background using pureblack
orwhite
colors.on
Heatmap
theconfig.label.fontSize
prop is replaced byconfig.label.minFontSize
andconfig.label.maxFontSize
. You can specify the same value for both properties to have a fixed font size.The
config.label.align
andconfig.label.baseline
props are removed from theHeatmapConfig
object. These props handle the alignment of the text within the text and they are not currently compatible with the font size scaling feature introduced. Is not very common to change the alignment of text within rectangular boxes where the centered option is the most used oneon
Heatmap
theonBrushEnd
props was moved from theconfig
to theSettings
. TheonBrushEndListener
prop signature was changed totype BrushEndListener = (brushEvent: XYBrushEvent | HeatmapBrushEvent) => void;
The public type varieties for domains are discontinued, in favor of retaining the single
DomainRange
export, which now has a mandatory{min: number, max: number}
. The developer can supplyNaN
where a finite min, max or both aren't defined (ie. in place of former effectiveundefined
)Release Notes
elastic/elastic-charts
37.0.0 (2021-10-05)
Bug Fixes
Code Refactoring
BREAKING CHANGES
DEFAULT_CHART_MARGINS
,DEFAULT_GEOMETRY_STYLES
,DEFAULT_CHART_PADDING
andDEFAULT_MISSING_COLOR
are no longer exposed as part of the APIDomainRange
export, which now has a mandatory{min: number, max: number}
. The developer can supplyNaN
where a finite min, max or both aren't defined (ie. in place of former effectiveundefined
). In addition, some console.warn punctuations changedCo-authored-by: Marco Vettorello [email protected]
Co-authored-by: Nick Partridge [email protected]
36.0.0 (2021-09-15)
Features
BREAKING CHANGES
35.0.0 (2021-09-13)
Bug Fixes
Features
BREAKING CHANGES
showDuplicatedTicks
causes a duplication check on the actual axis tick label (possibly yielded byAxis.tickLabel
rather than the more generaltickFormat
)config.label.fontSize
prop is replaced byconfig.label.minFontSize
andconfig.label.maxFontSize
. You can specify the same value for both properties to have a fixed font size. Theconfig.label.align
andconfig.label.baseline
props are removed from theHeatmapConfig
object.