Combine columns #42021

romeovs · 2024-04-30T10:11:50Z

Merges the feature branch combine-columns into master. (Epic here).

This branch contains work from: #40082, unaltered, except for fixing merge conflicts.

Closes #39980

Adds combine column drill through to the chill mode header columns.

How to verify

Open the Accounts table in chill mode
Click the header of a string column (eg. Email, First Name or Plan)
Click + Combine columns
Play around with the column editor
Click Done
Verify the new column is added as the last column

Demo

See video attached to this Slack message

This is needed to show the previews in the UX for the combining columns epic #39977. Fixes #39979.

This is following the original plan of using a drill for this. It's hard to combine that with the "preview" functionality. See [thread](https://metaboat.slack.com/archives/C06P22KS4JH/p1712264174056569) for discussion of how we might approach that issue. Fixes #39978.

… combine-columns

This is following the original plan of using a drill for this. It's hard to combine that with the "preview" functionality. See [thread](https://metaboat.slack.com/archives/C06P22KS4JH/p1712264174056569) for discussion of how we might approach that issue. Fixes #39978.

This is needed to show the previews in the UX for the combining columns epic #39977. Fixes #39979.

This reverts commit dac17f8, reversing changes made to 2934fab.

This reverts commit 2934fab, reversing changes made to 6a43c22.

This reverts commit 6a43c22, reversing changes made to 2257333.

* Type Section['icon'] properly, including the hack for summarize section * Add combine columns section * Add types for combine columns drill * Add combine columns drill component stub * Mock combine columns drill * Add component stub * Encapsulate typing hacks * Extract CombineColumnsDrill * Extract ColumnAndSeparatorRow * Improve naming, add button to edit separators * Translate string * Improve styling * Separator input * Handle removing columns * Rename lib.ts to utils.ts * Implement onSubmit * Add getNextColumnAndSeparator * Use form for a11y * Handle vertical overflow * Handle horizontal overflow * Extract formatSeparator * Use proper translations * Adjust min/max sizes * Fix scrollbar being unnecessarily shown * Display source column name * Avoid serializing the entire column and putting it into DOM * Use module.css extension * Format code * Improve a11y * Mock combineColumnsDrillExpression and previewExpression * Add Preview component * Move preview logic to Preview component * Style Preview * Format code * Move styles to CSS modules * Update UI to latest designs - move "Add another column" button - remove "Separated by X" button - always show separator inputs * Remove displayInfo mock * Add asReturned * Update CombineColumnsDrillThruInfo * Integrate new way of working with combine columns drill * Fix generating expression names * Integrate drill with click actions * Integrate new preview workflow * Revert "Update UI to latest designs" This reverts commit cda039d. * Use previewExpression interface correctly * Use new preview_expression API * Make sure columns are created with original query and stage index * Extract usePreview and handle preview errors * Update popover title * Move add column button * Fix outline being cut off * Style preview label as per design * Format empty separator * Make preview scrollable * Use project convention * Simplify ScrollArea usage * Use ScrollArea in CombineColumnsDrill * Update comment * Remove Lib.previewExpression * Clean up the preview after removing Lib.previewExpression * Implement getPreview * Change default column preview to 'text' * Remove usePreview hook which is no longer in use * Rename Preview to Example * Use different placeholder for separator input * Add whitespace placeholder * Add ColumnPicker boilerplate * Export Input * Use QueryColumnPicker for ColumnPicker * Pass width to AccordionList * Add sequences popover events with tippy * Remove unused ColumnOption helpers * Add testid to Example * Add e2e test for combine column in header * Select content of separator input on focus * Reduce padding on example * Add monospace variant to Text * Use monospace variant in Example * Use monospace font in textinput * Match faux-select label styles to our TextInput label styles * Add missing color * Remove !important * Use pre whitespace so consecutive spaces show up * Remove empty default * Add example for boolean * Remove !important * Use should.have text over contain --------- Co-authored-by: Romeo Van Snick <romeo@romeovansnick.be>

github-actions · 2024-04-30T10:14:36Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 912b5e5...5dbf1af.

Notify	File(s)
@alxnddr	frontend/src/metabase/visualizations/components/ClickActions/utils.ts frontend/src/metabase/visualizations/types/click-actions.ts
@kdoh	frontend/src/metabase/ui/components/overlays/Popover/index.tsx frontend/src/metabase/ui/index.ts

replay-io · 2024-04-30T10:33:55Z

Status	Complete ↗︎
Commit	`5dbf1af`
Results	⚠️ 5 Flaky ✅ 2445 Passed

src/metabase/lib/schema/drill_thru.cljc

ranquild

Inline comments. I'll test the branch locally a bit later today

ranquild · 2024-04-30T12:45:47Z

...ng/utils/drills/combine-columns-drill/components/CombineColumnsDrill/CombineColumnsDrill.tsx

+        <Title
+          mb="lg"
+          order={4}
+        >{jt`Combine “${columnInfo.displayName}” with other columns`}</Title>


Just t? There are no react nodes

ranquild · 2024-04-30T12:46:55Z

...etabase/querying/utils/drills/combine-columns-drill/components/ColumnPicker/ColumnPicker.tsx

+  onChange: (column: Lib.ColumnMetadata) => void;
+};
+
+const theme = getThemeOverrides();


Strong no. This should never be used directly.

I followed this pattern because it is already in use in the codebase.

I've change the code to use useMantineTheme but I had to export it from metabase/ui because it wasn't yet.

Or do you have another preferred way of reading theme properties?

useMantineTheme LGTM

ranquild · 2024-04-30T12:47:57Z

...etabase/querying/utils/drills/combine-columns-drill/components/ColumnPicker/ColumnPicker.tsx

+      return;
+    }
+    if (!evt.currentTarget.contains(evt.relatedTarget as Node)) {
+      setTimeout(() => setOpen(false), 100);


This will cause issues with onmounts before timeout. Please use a hook (maybe from react-use) that handles that.

ranquild · 2024-04-30T12:49:01Z

...etabase/querying/utils/drills/combine-columns-drill/components/ColumnPicker/ColumnPicker.tsx

+
+// Hack to prevent parent TippyPopover from closing when selecting an item
+// TODO: remove when TippyPopover is no longer used
+function Dropdown({


This should be handled by Popover already.

@romeovs please move this to metabase/ui like for Select

metabase/frontend/src/metabase/ui/components/inputs/Select/SelectDropdown/SelectDropdown.tsx

Line 8 in 2aa3cec

export const SelectDropdown = forwardRef(function SelectDropdown(

so people don't need to copy-paste this

ranquild · 2024-04-30T12:49:29Z

...tils/drills/combine-columns-drill/components/ColumnAndSeparatorRow/ColumnAndSeparatorRow.tsx

+  onRemove: (index: number) => void;
+}
+
+const { fontFamilyMonospace } = getThemeOverrides();


ranquild

@romeovs please use this to make the newly created column appear after the selected one

metabase/frontend/src/metabase/querying/utils/drills/column-extract-drill/column-extract-drill.tsx

Line 24 in 8e111ab

extra: () => ({ settingsSyncOptions: { column: clicked.column } }),

ranquild

@romeovs last comment about moving the hack for Popover to metabase/ui. Approving to unblock the PR

romeovs · 2024-05-01T13:08:50Z

@romeovs please use this to make the newly created column appear after the selected one

metabase/frontend/src/metabase/querying/utils/drills/column-extract-drill/column-extract-drill.tsx

Line 24 in 8e111ab

extra: () => ({ settingsSyncOptions: { column: clicked.column } }),

The project actually specifies to attach the column at the end. @mngr what is your preference here?

bshepherdson and others added 14 commits April 4, 2024 17:43

[MLv2] Add preview-expression to eval an expression on sample data

8a21d4c

This is needed to show the previews in the UX for the combining columns epic #39977. Fixes #39979.

Merge remote-tracking branch 'origin/mblib-combine-column-drill' into…

ee3c5c4

… combine-columns

Merge branch 'mblib-combine-column-drill' into combine-columns

2257333

Merge branch 'mblib-preview-expressions' into combine-columns

6a43c22

[MLv2] Add preview-expression to eval an expression on sample data

7734f0f

This is needed to show the previews in the UX for the combining columns epic #39977. Fixes #39979.

Merge branch 'mblib-preview-expressions' into combine-columns

2934fab

[MLv2] Add preview-expression to eval an expression on sample data

acd5824

This is needed to show the previews in the UX for the combining columns epic #39977. Fixes #39979.

Merge branch 'mblib-preview-expressions' into combine-columns

dac17f8

Revert "Merge branch 'mblib-preview-expressions' into combine-columns"

0affa17

This reverts commit dac17f8, reversing changes made to 2934fab.

Revert "Merge branch 'mblib-preview-expressions' into combine-columns"

55bf8f9

This reverts commit 2934fab, reversing changes made to 6a43c22.

Revert "Merge branch 'mblib-preview-expressions' into combine-columns"

08331d8

This reverts commit 6a43c22, reversing changes made to 2257333.

romeovs self-assigned this Apr 30, 2024

romeovs requested a review from camsaul as a code owner April 30, 2024 10:11

romeovs added the no-backport Do not backport this PR to any branch label Apr 30, 2024

metabase-bot bot added the visual Run Percy visual testing label Apr 30, 2024

romeovs mentioned this pull request Apr 30, 2024

"Combine columns" drill thru #42019

Closed

Merge branch 'master' into combine-columns

f529c5d

romeovs force-pushed the combine-columns branch from 849a6e2 to f529c5d Compare April 30, 2024 10:21

nemanjaglumac requested a review from a team April 30, 2024 12:01

nemanjaglumac added the .Team/QueryingComponents label Apr 30, 2024

nemanjaglumac reviewed Apr 30, 2024

View reviewed changes

src/metabase/lib/schema/drill_thru.cljc Outdated Show resolved Hide resolved

nemanjaglumac requested review from ranquild and a team April 30, 2024 12:27

Remove duplicated code

916b615

ranquild reviewed Apr 30, 2024

View reviewed changes

romeovs added 4 commits April 30, 2024 17:37

Export useMantineTheme from metabase/ui

c4cb323

Use useMantineTheme instead of reading theme directly

5a8212c

Remove setTimeout in popover

116c941

Use plain t instead of jt

c55da8e

romeovs requested a review from ranquild May 1, 2024 07:07

ranquild approved these changes May 1, 2024

View reviewed changes

Move Popover dropdown sequence hack to metabase/ui

5dbf1af

romeovs merged commit 0dc7504 into master May 1, 2024
112 checks passed

romeovs deleted the combine-columns branch May 1, 2024 16:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine columns #42021

Combine columns #42021

romeovs commented Apr 30, 2024 •

edited

Loading

github-actions bot commented Apr 30, 2024 •

edited

Loading

replay-io bot commented Apr 30, 2024 •

edited

Loading

ranquild left a comment

ranquild Apr 30, 2024

ranquild Apr 30, 2024

romeovs Apr 30, 2024

ranquild Apr 30, 2024

ranquild Apr 30, 2024

ranquild Apr 30, 2024

ranquild May 1, 2024

ranquild Apr 30, 2024

ranquild left a comment

ranquild left a comment

romeovs commented May 1, 2024

Combine columns #42021

Combine columns #42021

Conversation

romeovs commented Apr 30, 2024 • edited Loading

How to verify

Demo

github-actions bot commented Apr 30, 2024 • edited Loading

replay-io bot commented Apr 30, 2024 • edited Loading

ranquild left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranquild left a comment

Choose a reason for hiding this comment

ranquild left a comment

Choose a reason for hiding this comment

romeovs commented May 1, 2024

romeovs commented Apr 30, 2024 •

edited

Loading

github-actions bot commented Apr 30, 2024 •

edited

Loading

replay-io bot commented Apr 30, 2024 •

edited

Loading