Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

Fix D3 format for some viz types #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spotx-ajacobi
Copy link

Some viz types don't honor the predefined D3 formats users enter for metrics. This change fixes this for a handful of viz types.

 modified:   packages/superset-ui-legacy-preset-chart-big-number/src/BigNumber/transformProps.js
 modified:   packages/superset-ui-legacy-preset-chart-nvd3/src/transformProps.js

JIRA: SUP-146
@spotx-ajacobi spotx-ajacobi requested a review from a team as a code owner August 9, 2019 15:33
@ghost
Copy link

ghost commented Aug 9, 2019

There were the following issues with this Pull Request

  • Commit: 120a3ed
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@@ -82,7 +82,18 @@ export default function transformProps(chartProps) {
className = 'negative';
}

const formatValue = getNumberFormatter(yAxisFormat);
// SUP-146
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove comment

// SUP-146
let metricFormat = yAxisFormat;
if (!yAxisFormat) {
for (let x = 0; x < chartProps.datasource.metrics.length; x++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to check if datasource exists and metrics exist

if (!yAxisFormat) {
for (let x = 0; x < chartProps.datasource.metrics.length; x++) {
if (chartProps.datasource.metrics[x].metric_name == metric) {
metricFormat = chartProps.datasource.metrics[x].d3format;
Copy link
Collaborator

Choose a reason for hiding this comment

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

d3format may be not set

let d3YAxis2Format;
if (chartProps.formData.vizType == "pie") {
for (let x = 0; x < chartProps.datasource.metrics.length; x++) {
if (chartProps.datasource.metrics[x].metric_name == chartProps.formData.metric) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like the logic here can be extracted into a function?

@rusackas
Copy link
Member

rusackas commented Dec 4, 2019

Picked up the ball on this over here:
#280

nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
…rset#171)

* feat: add parseLength function

* feat: export

* fix: address Kim's comment
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants