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

Visualizations customizable settings #4793

Merged
merged 14 commits into from
Apr 25, 2020

Conversation

gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented Apr 9, 2020

What type of PR is this? (check all applicable)

  • Other

Description

Reorganize visualization components folder
Components related to Redash app were removed from visualizations/components folder (EditVisualizationDialog and VisualizationRenderer). Also created a general Renderer and Editor that takes the type and select it from the list.

Add customizable settings
Creates a mutable object with options (currently only one from the HelpTrigger, soon others from clientConfig options).

Update HelpTrigger
HelpTrigger now alternatively receives a link and a title in replacement for the type constant. Also it renders a regular <a> when link is not inside redash.io (adds an External icon in such case).

clientConfig options
Were all put into visualization settings.

Related Tickets & Documents

--

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

--

@gabrieldutra gabrieldutra force-pushed the separate-visualization-components branch from 77c0877 to 7335d45 Compare April 13, 2020 13:38
@gabrieldutra gabrieldutra marked this pull request as ready for review April 13, 2020 22:40
@@ -42,12 +43,17 @@ export default function VisualizationRenderer(props) {
setFilters(combineFilters(filtersRef.current, props.filters));
}, [props.filters]);

const cleanColumnNames = useMemo(
() => map(data.columns, col => ({ ...col, name: getColumnCleanName(col.friendly_name) })),
Copy link
Member Author

Choose a reason for hiding this comment

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

@kravets-levko it seemed to make sense to move getColumnCleanName out of Table vis and rather apply it to all visualizations, do you see any problems may happen with this change? (that method is used to remove ::filter from column names for example)

Comment on lines +15 to +24
choroplethAvailableMaps: {
countries: {
name: "Countries",
url: countriesDataUrl,
},
subdiv_japan: {
name: "Japan/Prefectures",
url: subdivJapanDataUrl,
},
},
Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to go in the direction of #4599 as it has a complementary code to this.

@@ -1,25 +1,16 @@
import { omit, merge } from "lodash";
import { omit, merge, get } from "lodash";
import axios from "axios";
Copy link
Member Author

Choose a reason for hiding this comment

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

No reason to add a service axios dependency instead of using the base one.

@gabrieldutra gabrieldutra merged commit 60bc1f8 into master Apr 25, 2020
@gabrieldutra gabrieldutra deleted the separate-visualization-components branch April 25, 2020 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants