fix(table chart): Show Cell Bars correctly

[SA-Ark]

SUMMARY

The issue here is that the “isMetric” key for percentage metric is coming up as False from the columns when it is being imported from props.

As we follow the data into the line where we are creating “const valueRange”, we can see that we are calling the getValueRange function only when either “isMetric” is True or “isRawRecords” is true. “isRawRecords” is false for both, and “isMetric” is only true for regular metrics and not percentage metrics. So the "getValueRange" function is not even being called for percentage metrics which is why it is not calculated for these metrics.

When we follow the data further, we can see in the “” component where we are returning the Cell Bars we have a condition of “valueRange” to not be null in order to display the cell bars. We can verify with console.logs that the valueRange is never calculated in the case of percentage metrics and it is null. This is why the cell bars are not showing up for only the percentage metrics because the css class for it is not being added since “valueRange” is null.

Proposed solution:

A solution that fixed the cell bars for me is adding a separate key[0] == “%” in the conditional where we are checking (isMetric || isRawRecords) while creating the const “valueRange” variable. This ensures none of the other code is messed with and the cell bars still show up.

Alternatively, we can also make the isMetric variable true from the file where the percentage metric props are being created, but I am not sure if that might break other parts of the code, so this seemed to be the least intrusive way to fix it.

End result:

Before:

After:

TESTING INSTRUCTIONS

Create a table chart and add both regular metrics and percentage metrics and enable the show cell bars options. They both should be showing when it previously was not.

ADDITIONAL INFORMATION

Fixes #24495 - Show Cell Bars does not work on the Percentage Metrics in Table Chart

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[github-actions]

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

[geido]

Hey @SA-Ark you got a lint issue to fix. You can run npm run lint-fix

[michael-s-molina]

Thank you for the PR @SA-Ark. I left some first-pass comments.

[michael-s-molina]

Suggested change

.aider*

[michael-s-molina]

It's a good practice to have a separate PR for bumping a package unless it's related to the fix. This practice facilitates reverting only the package bump if needed.

Suggested change

"@types/mousetrap": "^1.6.12",

[michael-s-molina]

DataColumnMeta has a isPercentMetric property.

Suggested change

	(isMetric || isRawRecords || key[0] === '%') &&
	(isMetric || isRawRecords || isPercentMetric) &&