You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "SA-Ark (via GitHub)" <gi...@apache.org> on 2023/10/19 12:07:51 UTC
[PR] fix(table chart): Show Cell Bars correctly #25625 [superset]
SA-Ark opened a new pull request, #25707:
URL: https://github.com/apache/superset/pull/25707
### 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 “<StyledCell>” 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:
- [X] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(table chart): Show Cell Bars correctly #25625 [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina merged PR #25707:
URL: https://github.com/apache/superset/pull/25707
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org