feat(plugin-chart-table): add column config control #1019

ktmud · 2021-03-22T17:08:14Z

🏆 Enhancements

Add column formatting control to table chart, allowing users to change things like column width, text align, number/date formatting per column, all in the Explore view itself (previously you are able to set d3 formats per column, but you'd have to do it via calculated columns or saved metrics).

Also added a separate formatting config for small numbers, so that they can be display as 0 when needed. This is useful when you want to apply human readable SI prefix to big numbers, but don't want to add that to small numbers. When there are both small (abs(x) < 1) and large numbers, users often confuse m for 10^-3 with M for 10³ (e.g. $1.3m is actually $0.0013, not $1,300,000, the latter would be formatted as $1.3M). By having a separate formatting for small numbers, users can then decide whether to display them as 0 or how many significant digits to use for them.

Must used together with apache/superset#13758 , which passed query responses to chart controls.

After

column.formatting.demo.mp4

Before

vercel · 2021-03-22T17:08:16Z

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/7YwTThF26FAcKh31pR5iXHr92sxB
✅ Preview: https://superset-ui-git-table-column-formatting-superset.vercel.app

codecov · 2021-03-25T07:46:06Z

Codecov Report

Merging #1019 (2321007) into master (cdd6990) will decrease coverage by 0.24%.
The diff coverage is 20.52%.

@@            Coverage Diff             @@
##           master    #1019      +/-   ##
==========================================
- Coverage   27.93%   27.68%   -0.25%     
==========================================
  Files         417      427      +10     
  Lines        8608     8747     +139     
  Branches     1242     1311      +69     
==========================================
+ Hits         2405     2422      +17     
- Misses       6043     6152     +109     
- Partials      160      173      +13

Impacted Files	Coverage Δ
...-chart-controls/src/components/ColumnTypeLabel.tsx	`59.09% <0.00%> (-17.38%)`	⬇️
...ols/src/components/ControlForm/ControlFormItem.tsx	`0.00% <0.00%> (ø)`
...hart-controls/src/components/ControlForm/index.tsx	`0.00% <0.00%> (ø)`
...ui-chart-controls/src/components/ControlHeader.tsx	`0.00% <0.00%> (ø)`
...erset-ui-chart-controls/src/components/Tooltip.tsx	`20.00% <ø> (ø)`
packages/superset-ui-chart-controls/src/index.ts	`100.00% <ø> (ø)`
...omponents/ColumnConfigControl/ColumnConfigItem.tsx	`0.00% <0.00%> (ø)`
...onents/ColumnConfigControl/ColumnConfigPopover.tsx	`0.00% <0.00%> (ø)`
.../shared-controls/components/RadioButtonControl.tsx	`0.00% <0.00%> (-9.10%)`	⬇️
...et-ui-chart-controls/src/shared-controls/index.tsx	`38.46% <ø> (-2.60%)`	⬇️
... and 29 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 cdd6990...2321007. Read the comment docs.

kristw · 2021-03-25T17:23:35Z

For the text alignment, can it support center?
Perhaps can use icon if possible.

kristw

halfway through

kristw · 2021-03-25T17:26:26Z

plugins/plugin-chart-echarts/src/Graph/controlPanel.tsx

-                  label: 'circular',
-                  value: 'circular',
-                },
+                ['force', t('Force')],


Is this related to the table?

I changed the signature of the RadioButtonControl to make it more consistent with the SelectControl.

kristw · 2021-03-25T17:30:40Z

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

-          const [isHtml, text, customClassName] = formatValue(column, value);
-          const style = {
+          let rounded = value;
+          if (fractionDigits !== undefined && isNumber && typeof value === 'number') {


fractionDigits !== undefined && isNumber can be computed outside of function.

I can move isNumber out, fractionDigits !== undefined would still be needed here since it can also be zero and I am using its real value later.

kristw · 2021-03-25T17:31:13Z

plugins/plugin-chart-table/src/utils/formatValue.ts

-): [boolean, string, string | null] {
+): [boolean, string] {
  if (value === null) {
-    return [false, 'N/A', 'dt-is-null'];


Do we no longer need this flag?

Yes, it is moved to inline code as I realized this is not very generalized for formatValue.

https://github.com/apache-superset/superset-ui/pull/1019/files/6f4cca25f8c003631d2e853ee252d32b7dcf8f54#diff-1b5ecf1086dbd4b0e942970dc4e4b20077a2d30d6046d08444b46d8cb57de2a4R271-R275

Also tried to use inline Emotion's css prop but thought manually constructed css classNames are still faster, especially that we need to set it for every cell.

kristw · 2021-03-25T17:42:15Z

packages/superset-ui-chart-controls/src/components/ControlForm/controls.tsx

+   */
+  debounceDelay?: number;
+} & (T extends 'Select'
+  ? {


Would it worth extracting the type definition after ? into named type?

I don't think it's worth it as you'd have to name them and reference them. The indirectness means you'd have to jump back and forth when glancing through code. I also don't see much added value in terms of readability.

kristw · 2021-03-25T17:51:43Z

...ui-chart-controls/src/shared-controls/components/ColumnConfigControl/ColumnConfigControl.tsx

+  const [showAllColumns, setShowAllColumns] = useState(false);
+
+  const getColumnInfo = (col: string) => columnConfigs[col] || {};
+  const setColumnConfig = (col: string, config: T) => {


useCallback?

I don't think this matters much in practice. This is only used in an event handler---onChange of ColumnConfigItem, which shouldn't trigger the component's re-render if it's the only prob being updated.

kristw · 2021-03-25T17:52:37Z

packages/superset-ui-chart-controls/src/components/ColumnOption.tsx

 import React from 'react';
 import { Tooltip } from './Tooltip';
-import { ColumnTypeLabel } from './ColumnTypeLabel';
+import { ColumnTypeLabel, LaxColumnType } from './ColumnTypeLabel';


What is Lax?

Like less strict... Supposedly column types should be mutually exclusive, but we also have some more random types from different places. LaxColumnType makes sure the column type icon component can accept all these types, while more strict ColumnType will be used elsewhere.

junlincc · 2021-03-25T23:32:33Z

This is great improvement, thank you Jesse! Are you planning to implement more enhancements to table chart?
@mihir174 let's revisit the control layout and popover when both tables are done.

cc @kgabryje @zhaoyongjie

ktmud · 2021-03-26T06:21:45Z

For the text alignment, can it support center?
Perhaps can use icon if possible.

Updated to fontawesome icons.

ktmud · 2021-03-26T20:43:02Z

@junlincc I don't have any other major work planned for table viz.

kgabryje · 2021-03-26T20:47:36Z

@junlincc I don't have any other major work planned for table viz.

After this PR is merged, I'll work on implementing totals if you don't mind

junlincc · 2021-03-26T20:55:40Z

@kristw Hey Krist, is this PR ready to move forward? we are happy to take it from here. 🙏

kristw · 2021-03-27T00:13:27Z

@kristw Hey Krist, is this PR ready to move forward? we are happy to take it from here. 🙏

Thanks. I think I'm good with the current.

ktmud · 2021-03-27T00:53:03Z

Merging and will do a follow up PR if there are any bugs surfacing up while testing.

…1019)

junlincc · 2021-04-29T05:46:01Z

Is the expected behavior - stretch to fit fit screen by default? @ktmud

Screen.Recording.2021-04-28.at.10.48.37.PM.mov

junlincc · 2021-04-29T06:33:41Z

Currency format only support $, we may wanna expand support for top 5 currency. add it to backlog

junlincc · 2021-04-29T06:50:27Z

A couple more issues..

SHOW CELL BARS is check by default in the popover, but it's not applied to the column by default, user needs to uncheck then check to see changes
Without re-checking the SHOW CELL BARS box first, ALIGN +/- AND COLOR +/- doesn't really work.

Screen.Recording.2021-04-28.at.11.47.20.PM.mov

pull-request-size bot added the size/XXL label Mar 22, 2021

ktmud mentioned this pull request Mar 23, 2021

feat(explore): adhoc column formatting for Table chart apache/superset#13758

Merged

6 tasks

vercel bot had a problem deploying to Preview March 25, 2021 07:14 Failure

ktmud changed the title ~~[WIP] feat(plugin-chart-table): add column config control~~ feat(plugin-chart-table): add column config control Mar 25, 2021

ktmud marked this pull request as ready for review March 25, 2021 07:14

ktmud requested a review from a team as a code owner March 25, 2021 07:14

ktmud added 4 commits March 25, 2021 00:26

feat(plugin-chart-table): add column config control

bb3bd9f

Apply configs and faster debounce

528b748

Implement all formats

c07f9f8

fix build

6f4cca2

ktmud force-pushed the table-column-formatting branch from be03bb6 to 6f4cca2 Compare March 25, 2021 07:33

vercel bot deployed to Preview March 25, 2021 07:33 View deployment

ktmud added the ready-for-review label Mar 25, 2021

kristw reviewed Mar 25, 2021

View reviewed changes

junlincc requested a review from zhaoyongjie March 25, 2021 23:28

Replace alignment with icons

2321007

vercel bot deployed to Preview March 26, 2021 00:27 View deployment

kristw approved these changes Mar 27, 2021

View reviewed changes

ktmud merged commit 2af6a24 into master Mar 27, 2021

delete-merged-branch bot deleted the table-column-formatting branch March 27, 2021 00:52

junlincc removed the ready-for-review label Mar 27, 2021

This was referenced Mar 27, 2021

feat(plugin-chart-table): add small number formatter #1028

Merged

fix(chart-controls): export/import DTTM_ALIAS properly #1035

Merged

[Chart] Create Andon chart apache-superset/superset-roadmap#159

Closed

NejcZdovc pushed a commit to blotoutio/superset-ui that referenced this pull request Apr 20, 2021

feat(plugin-chart-table): add column config control (apache-superset#…

a3891c6

…1019)

mayurnewase mentioned this pull request May 6, 2021

feat(plugin-chart-echart): New Tree chart #1018

Merged

feat(plugin-chart-table): add column config control #1019

feat(plugin-chart-table): add column config control #1019

Uh oh!

Conversation

ktmud commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

After

Before

Uh oh!

vercel bot commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kristw commented Mar 25, 2021

Uh oh!

kristw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junlincc commented Mar 25, 2021

Uh oh!

ktmud commented Mar 26, 2021

Uh oh!

ktmud commented Mar 26, 2021

Uh oh!

kgabryje commented Mar 26, 2021

Uh oh!

junlincc commented Mar 26, 2021

Uh oh!

kristw commented Mar 27, 2021

Uh oh!

ktmud commented Mar 27, 2021

Uh oh!

junlincc commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junlincc commented Apr 29, 2021

Uh oh!

junlincc commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ktmud commented Mar 22, 2021 •

edited

Loading

vercel bot commented Mar 22, 2021 •

edited

Loading

codecov bot commented Mar 25, 2021 •

edited

Loading

junlincc commented Apr 29, 2021 •

edited

Loading

junlincc commented Apr 29, 2021 •

edited

Loading