Skip to content
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

[Bugfix] HeatmapHighChart: Prioritize configOverride over defaultOptions #1490

Merged
merged 4 commits into from
Jun 14, 2022

Conversation

ms-kashyap
Copy link
Collaborator

Note: Recreating PR #1457 with signed commits.

Description

Context

When trying to create a heatmap using HeatmapHighChart, I had set legend: { enabled : true } but I was not seeing the legend in the heatmap.

It turns out that lodash.merge prioritizes values from right to left. Therefore, we should specify configOverride last since we want its values to be prioritzed over defaultOptions.

Showcasing bug

Original code:
image
image

Showcasing fix

Fix (swap configOverride and defaultOptions):
image
image

Checklist

  • I have added screenshots above for all UI changes.
  • Documentation was updated if it was needed.
  • New tests were added or changes were manually verified.

@ms-kashyap ms-kashyap requested a review from romanlutz June 13, 2022 16:50
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2022

Codecov Report

Merging #1490 (b23f275) into main (a34530c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1490   +/-   ##
=======================================
  Coverage   89.30%   89.30%           
=======================================
  Files          38       38           
  Lines        1617     1617           
=======================================
  Hits         1444     1444           
  Misses        173      173           
Flag Coverage Δ
unittests 89.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a34530c...b23f275. Read the comment docs.

Copy link
Contributor

@romanlutz romanlutz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for recreating it!

@romanlutz romanlutz enabled auto-merge (squash) June 13, 2022 16:58
@romanlutz romanlutz merged commit e1f00d9 into main Jun 14, 2022
@romanlutz romanlutz deleted the ms-kashyap/highchartsheatmap branch June 14, 2022 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants