Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat(legacy-plugin-chart-markup): add controls to markup chart #479

Conversation

pkdotson
Copy link
Contributor

πŸ’” Breaking Changes

πŸ† Enhancements
Add controls to markup chart plugin.
πŸ“œ Documentation

πŸ› Bug Fix

🏠 Internal

@pkdotson pkdotson requested a review from a team as a code owner May 12, 2020 15:51
@vercel
Copy link

vercel bot commented May 12, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

πŸ” Inspect: https://vercel.com/superset/superset-ui/oi6jpdekm
βœ… Preview: https://superset-ui-git-fork-preset-io-phillip-so-506-markupc-066eff.superset.now.sh

import { validateNonEmpty } from '@superset-ui/validator';
import { formatSelectOptions } from '@superset-ui/control-utils';

console.log('what are going on with these controls')
Copy link
Contributor

Choose a reason for hiding this comment

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

remove console.log

],
},
],
sectionOverrides: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this section needed?

Copy link
Member

Choose a reason for hiding this comment

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

@kristw I think they are needed (or at least relevant). There are a few plugins that do this... iframe, markup, and separator. It appears this basically removes the time_range control that's seemingly injected automatically in some cases as part of the druidTimeSeries. Maybe we can find a better way of addressing this in the future, but it seems out of the scope of this PR, which is just to migrate the code that's there already.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it

@kristw kristw changed the title chore: add controls to markup chart feat(legacy-plugin-chart-markup): add controls to markup chart May 12, 2020
@codecov
Copy link

codecov bot commented May 12, 2020

Codecov Report

Merging #479 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #479      +/-   ##
==========================================
+ Coverage   22.36%   22.38%   +0.01%     
==========================================
  Files         266      271       +5     
  Lines        6549     6563      +14     
  Branches      605      608       +3     
==========================================
+ Hits         1465     1469       +4     
- Misses       5047     5057      +10     
  Partials       37       37              
Impacted Files Coverage Ξ”
...ins/legacy-plugin-chart-markup/src/controlPanel.js 0.00% <0.00%> (ΓΈ)
plugins/legacy-plugin-chart-markup/src/index.js 0.00% <ΓΈ> (ΓΈ)
plugins/legacy-preset-chart-nvd3/src/Bar/index.js 0.00% <0.00%> (ΓΈ)
plugins/legacy-preset-chart-nvd3/src/Pie/index.js 0.00% <0.00%> (ΓΈ)
plugins/legacy-plugin-chart-partition/src/index.js 0.00% <0.00%> (ΓΈ)
plugins/legacy-preset-chart-nvd3/src/Area/index.js 0.00% <0.00%> (ΓΈ)
plugins/legacy-preset-chart-nvd3/src/Line/index.js 0.00% <0.00%> (ΓΈ)
...ugins/legacy-preset-chart-nvd3/src/Bubble/index.js 0.00% <0.00%> (ΓΈ)
...ugins/legacy-preset-chart-nvd3/src/Bullet/index.js 0.00% <0.00%> (ΓΈ)
...gins/legacy-preset-chart-nvd3/src/Compare/index.js 0.00% <0.00%> (ΓΈ)
... and 9 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 f06cb4a...bbdbd49. Read the comment docs.

@rusackas rusackas merged commit 7801121 into apache-superset:master May 12, 2020
@rusackas rusackas deleted the phillip/SO-506-markupchart-plugin-controls-migration branch July 17, 2020 20:37
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