Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resizable Pivot Table Data Columns #27725

Merged
merged 23 commits into from
Jan 23, 2023
Merged

Resizable Pivot Table Data Columns #27725

merged 23 commits into from
Jan 23, 2023

Conversation

iethree
Copy link
Contributor

@iethree iethree commented Jan 17, 2023

resolves #27691

Description

Allow users to resize data/value columns manually. We do not even try to do any automated measuring of columns because there is a potentially enormous number of them.

Screenshots

resizeDataCols

rearrange dimensions

Testing Steps

Open an enormous, gross pivot table like this one:

http://localhost:3000/question#eyJkYXRhc2V0X3F1ZXJ5Ijp7ImRhdGFiYXNlIjoxLCJxdWVyeSI6eyJzb3VyY2UtdGFibGUiOjIsImFnZ3JlZ2F0aW9uIjpbWyJtYXgiLFsiZmllbGQiLDEzLG51bGxdXSxbImNvdW50Il0sWyJhdmciLFsiZmllbGQiLDEzLG51bGxdXV0sImJyZWFrb3V0IjpbWyJmaWVsZCIsMTQseyJ0ZW1wb3JhbC11bml0IjoicXVhcnRlciJ9XSxbImZpZWxkIiw4LHsic291cmNlLWZpZWxkIjo5fV0sWyJmaWVsZCIsMTIseyJiaW5uaW5nIjp7InN0cmF0ZWd5IjoiZGVmYXVsdCJ9fV0sWyJmaWVsZCIsMTcseyJiaW5uaW5nIjp7InN0cmF0ZWd5IjoiZGVmYXVsdCJ9fV1dfSwidHlwZSI6InF1ZXJ5In0sImRpc3BsYXkiOiJwaXZvdCIsImRpc3BsYXlJc0xvY2tlZCI6dHJ1ZSwicGFyYW1ldGVycyI6W10sInZpc3VhbGl6YXRpb25fc2V0dGluZ3MiOnsicGl2b3RfdGFibGUuY29sdW1uX3NwbGl0Ijp7InJvd3MiOltbImZpZWxkIiwxNCx7InRlbXBvcmFsLXVuaXQiOiJxdWFydGVyIn1dXSwiY29sdW1ucyI6W1siZmllbGQiLDEyLHsiYmlubmluZyI6eyJzdHJhdGVneSI6Im51bS1iaW5zIiwibWluLXZhbHVlIjowLCJtYXgtdmFsdWUiOjEwMCwibnVtLWJpbnMiOjgsImJpbi13aWR0aCI6MTIuNX19XSxbImZpZWxkIiw4LHsic291cmNlLWZpZWxkIjo5fV0sWyJmaWVsZCIsMTcseyJiaW5uaW5nIjp7InN0cmF0ZWd5IjoibnVtLWJpbnMiLCJtaW4tdmFsdWUiOjAsIm1heC12YWx1ZSI6NzAsIm51bS1iaW5zIjo4LCJiaW4td2lkdGgiOjEwfX1dXSwidmFsdWVzIjpbWyJhZ2dyZWdhdGlvbiIsMV0sWyJhZ2dyZWdhdGlvbiIsMl0sWyJhZ2dyZWdhdGlvbiIsMF1dfX0sIm9yaWdpbmFsX2NhcmRfaWQiOjIxfQ==
  • see that you can resize any column (when there are multiple column headers, only the bottom one should be resizable)
  • see that rearranging field dimensions in viz settings works properly, and resets column widths

Remaining work to be done:

Persist column sizes: #27598

@iethree iethree self-assigned this Jan 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2023

Notifying subscribers in CODENOTIFY files for diff 15ca786...836e91c.

Notify File(s)
@alxnddr frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx
frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTableCell.tsx
frontend/src/metabase/visualizations/visualizations/PivotTable/constants.ts
frontend/src/metabase/visualizations/visualizations/PivotTable/types.ts
frontend/src/metabase/visualizations/visualizations/PivotTable/utils.ts
frontend/src/metabase/visualizations/visualizations/PivotTable/utils.unit.spec.ts

@codecov
Copy link

codecov bot commented Jan 17, 2023

Codecov Report

Base: 66.15% // Head: 66.12% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (836e91c) compared to base (2553505).
Patch coverage: 60.46% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #27725      +/-   ##
==========================================
- Coverage   66.15%   66.12%   -0.04%     
==========================================
  Files        3249     3272      +23     
  Lines       94344    94806     +462     
  Branches    11975    12072      +97     
==========================================
+ Hits        62413    62688     +275     
- Misses      26963    27150     +187     
  Partials     4968     4968              
Flag Coverage Δ
back-end 85.31% <ø> (-0.01%) ⬇️
front-end 48.38% <60.46%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...lizations/visualizations/PivotTable/PivotTable.tsx 58.88% <28.57%> (-3.62%) ⬇️
...tions/visualizations/PivotTable/PivotTableCell.tsx 78.94% <50.00%> (-4.39%) ⬇️
.../visualizations/visualizations/PivotTable/utils.ts 87.35% <94.11%> (+0.68%) ⬆️
frontend/src/metabase/core/utils/arrays/arrays.ts 100.00% <100.00%> (ø)
...ualizations/visualizations/PivotTable/constants.ts 100.00% <100.00%> (ø)
...ry_builder/components/NativeQueryEditor.styled.tsx 0.00% <0.00%> (-100.00%) ⬇️
..._builder/components/NativeQueryEditor/constants.ts 0.00% <0.00%> (-100.00%) ⬇️
...w/PreviewQueryButton/PreviewQueryButton.styled.tsx 0.00% <0.00%> (-100.00%) ⬇️
...tor/RightClickPopover/RightClickPopover.styled.jsx 0.00% <0.00%> (-100.00%) ⬇️
...yEditorSidebar/NativeQueryEditorSidebar.styled.jsx 0.00% <0.00%> (-100.00%) ⬇️
... and 109 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Base automatically changed from gh14350-make-pivotTable-functional to master January 17, 2023 19:40
# Conflicts:
#	frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.tsx
#	frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTableCell.tsx
#	frontend/src/metabase/visualizations/visualizations/PivotTable/types.ts
#	frontend/src/metabase/visualizations/visualizations/PivotTable/utils.ts
@deploysentinel
Copy link

deploysentinel bot commented Jan 17, 2023

No failed tests 🎉

@iethree iethree changed the title Resizable Data Cols Resizable Pivot Table Data Columns Jan 17, 2023
@iethree iethree requested a review from a team January 20, 2023 16:56
Copy link
Member

@alxnddr alxnddr left a comment

Choose a reason for hiding this comment

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

Wdyt about the feasibility of testing this? Such DnD things are harder to write tests for. It would not be valuable to have a barely maintainable test just to formally have it covered if it involves a poorly testable code while we don't have the capacity to refactor. Although, maybe we can mock cells here to trigger onResize to verify updated widths? Or maybe a Cypress test would be more maintainable, what do you think?

@iethree
Copy link
Contributor Author

iethree commented Jan 23, 2023

Wdyt about the feasibility of testing this? Such DnD things are harder to write tests for. It would not be valuable to have a barely maintainable test just to formally have it covered if it involves a poorly testable code while we don't have the capacity to refactor. Although, maybe we can mock cells here to trigger onResize to verify updated widths? Or maybe a Cypress test would be more maintainable, what do you think?

column measurement in unit tests is pretty hacky, esp. since we don't have "real" pointer events. I added a good end-to end test that I think covers most of the cases here in the child branch: 9cb6f06fc7b47b9994416f974bb8d84dcb6511b1

@iethree iethree merged commit d0d0653 into master Jan 23, 2023
@iethree iethree deleted the gh14350-resizable-data-cols branch January 23, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow value pivot table columns to be resized
2 participants