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

fix(legacy-plugin-chart-table): time column formatting #340

Merged
merged 1 commit into from
Apr 3, 2020
Merged

fix(legacy-plugin-chart-table): time column formatting #340

merged 1 commit into from
Apr 3, 2020

Conversation

ktmud
Copy link
Contributor

@ktmud ktmud commented Apr 3, 2020 •
edited
Loading

🐛 Bug Fix

Table chart displays time column incorrectly when "include time" is selected. This PR fixes it. It also fixes a bunch of linting issues surfaced by migrating the plugin packages to the superset-ui repo.

Snip20200403_50

Screenshots

Before

Snip20200403_49

After

image

Test Plan

Added unit test.

@ktmud ktmud requested a review from a team as a code owner April 3, 2020 17:23
@vercel
Copy link

vercel bot commented Apr 3, 2020 •
edited
Loading

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

🔍 Inspect: https://zeit.co/superset/superset-ui/2mvh2os2p
✅ Preview: https://superset-ui-git-fork-ktmud-fix-table-time-format.superset.now.sh

@codecov
Copy link

codecov bot commented Apr 3, 2020 •
edited
Loading

Codecov Report

Merging #340 into master will decrease coverage by 0.34%.
The diff coverage is 41.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #340      +/-   ##
==========================================
- Coverage   92.01%   91.67%   -0.35%     
==========================================
  Files         118      118              
  Lines        1478     1489      +11     
  Branches      378      388      +10     
==========================================
+ Hits         1360     1365       +5     
- Misses         89       95       +6     
  Partials       29       29
Impacted Files Coverage Δ
plugins/table/test/testData.ts 100.00% <ø> (ø)
plugins/table/src/transformProps.ts 54.16% <18.18%> (-22.31%) ⬇️
plugins/table/src/ReactDataTable.tsx 80.48% <83.33%> (+2.28%) ⬆️

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 0e7a583...cca4716. Read the comment docs.

@netlify
Copy link

netlify bot commented Apr 3, 2020 •
edited
Loading

Deploy preview for superset-ui ready!

Built with commit cca4716

https://deploy-preview-340--superset-ui.netlify.com

@ktmud
Copy link
Contributor Author

ktmud commented Apr 3, 2020

@kristw This is a quick fix to address the "NaN"s. Do you think it's still worth porting in getTimeFormatterForGranularity for the Table chart after this fix?

@ktmud ktmud merged commit 6c7f710 into apache-superset:master Apr 3, 2020
@ktmud
Copy link
Contributor Author

ktmud commented Apr 3, 2020

Merging this quick for now. Will adapt formatter by granularity later.

@ktmud ktmud mentioned this pull request Apr 8, 2020
Merged
7 tasks
@ktmud ktmud deleted the fix-table-time-format branch April 8, 2020 17:44
kristw pushed a commit that referenced this pull request Apr 17, 2020
@dependabot-preview
build(deps): bump @storybook/addon-links from 5.0.11 to 5.3.13 (#340)
29b93d7 
Bumps [@storybook/addon-links](https://github.com/storybookjs/storybook/tree/HEAD/addons/links) from 5.0.11 to 5.3.13.
- [Release notes](https://github.com/storybookjs/storybook/releases)
- [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md)
- [Commits](https://github.com/storybookjs/storybook/commits/v5.3.13/addons/links)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kristw kristw kristw approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ktmud @kristw