feat(legacy-plugin-chart-pivot-table): add support for timestamp format #734

villebro · 2020-08-12T14:02:03Z

🏆 Enhancements

This adds support for formatted dates on pivot-table. This is done by prefixing date values with __timestamp:, after which they can be identified and parsed into formattable date objects. In addition sorting is done based on the epoch value, meaning that the format won't affect the sorting order. Other changes:

Add date format
Move visual options to own tab (currently on Query pane)
use time grain for formatting if smart formatting is selected
make sorting of numeric values based on the parsed float value, not the text value

AFTER:

BEFORE:

vercel · 2020-08-12T14:02:08Z

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/b1g9da0a0
✅ Preview: https://superset-ui-git-fork-preset-io-villebro-pivot-date.superset.vercel.app

codecov · 2020-08-13T09:07:44Z

Codecov Report

Merging #734 into master will increase coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #734      +/-   ##
==========================================
+ Coverage   24.35%   24.38%   +0.02%     
==========================================
  Files         340      340              
  Lines        7637     7662      +25     
  Branches      929      935       +6     
==========================================
+ Hits         1860     1868       +8     
- Misses       5704     5721      +17     
  Partials       73       73

Impacted Files	Coverage Δ
.../legacy-plugin-chart-pivot-table/src/PivotTable.js	`0.00% <0.00%> (ø)`
...acy-plugin-chart-pivot-table/src/transformProps.js	`0.00% <0.00%> (ø)`
packages/superset-ui-core/src/utils/logging.ts	`100.00% <0.00%> (ø)`

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 d323ce8...89edfba. Read the comment docs.

rusackas · 2020-08-14T17:04:50Z

plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js

@@ -44,25 +45,37 @@ const propTypes = {
 };

 function PivotTable(element, props) {
-  const { data, height, columnFormats, numberFormat, numGroups, verboseMap } = props;
+  const { data, height, columnFormats, dateFormat, numberFormat, numGroups, verboseMap } = props;

  const { html, columns } = data;
  const container = element;
  const $container = $(element);


jQuery!? 😭

Unfortunately pivot table still uses jQuery.dataTables.

Hey, if it works it works! 😄

ktmud · 2020-08-14T17:27:38Z

plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js


  const { html, columns } = data;
  const container = element;
  const $container = $(element);
+  const dateFormatter =
+    dateFormat === 'smart_date' ? smartDateFormatter : getTimeFormatter(dateFormat);


getTimeFormatter('smart_date') should work just fine. But since we are here, can you update it to follow the same logic as the table chart?

superset-ui/plugins/plugin-chart-table/src/transformProps.ts

Lines 138 to 151 in d323ce8

// Use granularity for "Adaptive Formatting" (smart_date)

const timeFormat = format || tableTimestampFormat;

formatter = getTimeFormatter(timeFormat);

if (timeFormat === smartDateFormatter.id) {

if (isTimeColumn(key)) {

formatter = getTimeFormatterForGranularity(granularity);

} else if (format) {

formatter = getTimeFormatter(format);

} else {

// return the identity string when datasource level formatter is not set

// and table timestamp format is set to Adaptive Formatting

formatter = String;

}

}

It has fallbacks to d3-format in pre-defined columns and uses Time Grain for smart_date.

If the code was hard to follow for you, I can also update this later and refactor these logics into some kind of common utilities.

Thanks for this reference! I didn't have strong opinions here aka. I was just out to make this work. I'll digest the table chart logic and see if it makes sense for me to streamline here 👍

@ktmud I implemented a slightly modified version of your formatter that felt most suitable for this slightly simpler case. Does it make sense to you?

Looks good! Thanks for the update. Do you mind adding a datetime column to Storybook as well?

rusackas

@ktmud's comments are duly noted (and appreciated!)... I'll leave it up to @villebro to address them, but this looks functionally sufficient to warrant the green checkmark.

villebro · 2020-08-14T19:26:30Z

Thanks for your reviews @ktmud and @rusackas ! ❤️

kristw

I think @ktmud 's comment make sense. At minimum I would recommend updating this line.

const dateFormatter =
    dateFormat === 'smart_date' ? smartDateFormatter : getTimeFormatter(dateFormat);

to

const dateFormatter = getTimeFormatter(dateFormat);

villebro · 2020-08-14T19:33:37Z

Oh, one more thing @ktmud and @rusackas . Currently the sorting carets are broken (displays a black box, see the screenshots). I wasn't sure how to fix this, do you guys know how to make the icon work properly?

villebro · 2020-08-14T19:34:15Z

Thanks @kristw , I'll update accordingly 👍

ktmud · 2020-08-14T19:48:22Z

Oh, one more thing @ktmud and @rusackas . Currently the sorting carets are broken (displays a black box, see the screenshots). I wasn't sure how to fix this, do you guys know how to make the icon work properly?

I used react-icons in the table chart.

superset-ui/plugins/plugin-chart-table/src/TableChart.tsx

Lines 91 to 98 in d323ce8

    
           function SortIcon<D extends object>({ column }: { column: ColumnInstance<D> }) { 
        
             const { isSorted, isSortedDesc } = column; 
        
             let sortIcon = <FaSort />; 
        
             if (isSorted) { 
        
               sortIcon = isSortedDesc ? <FaSortDesc /> : <FaSortAsc />; 
        
             } 
        
             return sortIcon; 
        
           }

In the future we should probably move the SVG icons from superset-frontend to superset-ui so they can be reused by visualizations.

villebro · 2020-08-18T19:04:00Z

I used react-icons in the table chart.

superset-ui/plugins/plugin-chart-table/src/TableChart.tsx

Lines 91 to 98 in d323ce8

function SortIcon<D extends object>({ column }: { column: ColumnInstance<D> }) {

const { isSorted, isSortedDesc } = column;

let sortIcon = <FaSort />;

if (isSorted) {

sortIcon = isSortedDesc ? <FaSortDesc /> : <FaSortAsc />;

}

return sortIcon;

}

In the future we should probably move the SVG icons from superset-frontend to superset-ui so they can be reused by visualizations.

@ktmud I was unsure how to apply this snippet to this code, as it doesn't seem to be natively React. Perhaps it's best if we merge this as-is and leave this fix for later?

ktmud · 2020-08-18T22:01:14Z

@ktmud I was unsure how to apply this snippet to this code, as it doesn't seem to be natively React. Perhaps it's best if we merge this as-is and leave this fix for later?

Sounds good

feat(legacy-plugin-chart-pivot-table): add support for timestamp format

2f2214d

villebro requested a review from a team as a code owner August 12, 2020 14:02

pull-request-size bot added the size/M label Aug 12, 2020

vercel bot deployed to Preview August 12, 2020 14:02 View deployment

add @superset-ui/time-format to deps

ff2e241

vercel bot deployed to Preview August 13, 2020 08:58 View deployment

rusackas reviewed Aug 14, 2020

View reviewed changes

ktmud reviewed Aug 14, 2020

View reviewed changes

rusackas approved these changes Aug 14, 2020

View reviewed changes

kristw approved these changes Aug 14, 2020

View reviewed changes

add granularity formatter and apply conditional date formatting to cells

c8e86c4

pull-request-size bot added size/L and removed size/M labels Aug 18, 2020

vercel bot deployed to Preview August 18, 2020 08:15 View deployment

flip if order

89edfba

vercel bot deployed to Preview August 18, 2020 08:46 View deployment

villebro merged commit 0acc730 into apache-superset:master Aug 19, 2020

villebro deleted the villebro/pivot-date branch August 19, 2020 06:20

villebro mentioned this pull request Aug 19, 2020

feat(viz-plugins): add date formatting to pivot-table apache/superset#10637

Merged

6 tasks

This was referenced Dec 15, 2021

[Snyk] Upgrade @superset-ui/chart from 0.12.22 to 0.14.23 TeliaDK/superset-ui#3

Open

[Snyk] Upgrade @superset-ui/query from 0.12.23 to 0.14.23 TeliaDK/superset-ui#4

Open

[Snyk] Upgrade @superset-ui/color from 0.12.21 to 0.14.23 TeliaDK/superset-ui#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(legacy-plugin-chart-pivot-table): add support for timestamp format #734

feat(legacy-plugin-chart-pivot-table): add support for timestamp format #734

villebro commented Aug 12, 2020 •

edited

Loading

vercel bot commented Aug 12, 2020 •

edited

Loading

codecov bot commented Aug 13, 2020 •

edited

Loading

rusackas Aug 14, 2020

ktmud Aug 14, 2020

villebro Aug 14, 2020

ktmud Aug 14, 2020

ktmud Aug 14, 2020

villebro Aug 14, 2020

villebro Aug 18, 2020

ktmud Aug 18, 2020

rusackas left a comment

villebro commented Aug 14, 2020 •

edited

Loading

kristw left a comment

villebro commented Aug 14, 2020

villebro commented Aug 14, 2020

ktmud commented Aug 14, 2020

villebro commented Aug 18, 2020 •

edited

Loading

ktmud commented Aug 18, 2020

	// Use granularity for "Adaptive Formatting" (smart_date)
	const timeFormat = format \|\| tableTimestampFormat;
	formatter = getTimeFormatter(timeFormat);
	if (timeFormat === smartDateFormatter.id) {
	if (isTimeColumn(key)) {
	formatter = getTimeFormatterForGranularity(granularity);
	} else if (format) {
	formatter = getTimeFormatter(format);
	} else {
	// return the identity string when datasource level formatter is not set
	// and table timestamp format is set to Adaptive Formatting
	formatter = String;
	}
	}

feat(legacy-plugin-chart-pivot-table): add support for timestamp format #734

feat(legacy-plugin-chart-pivot-table): add support for timestamp format #734

Conversation

villebro commented Aug 12, 2020 • edited Loading

AFTER:

BEFORE:

vercel bot commented Aug 12, 2020 • edited Loading

codecov bot commented Aug 13, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rusackas left a comment

Choose a reason for hiding this comment

villebro commented Aug 14, 2020 • edited Loading

kristw left a comment

Choose a reason for hiding this comment

villebro commented Aug 14, 2020

villebro commented Aug 14, 2020

ktmud commented Aug 14, 2020

villebro commented Aug 18, 2020 • edited Loading

ktmud commented Aug 18, 2020

villebro commented Aug 12, 2020 •

edited

Loading

vercel bot commented Aug 12, 2020 •

edited

Loading

codecov bot commented Aug 13, 2020 •

edited

Loading

villebro commented Aug 14, 2020 •

edited

Loading

villebro commented Aug 18, 2020 •

edited

Loading