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

feat: truncate long values in table viz, a per-column setting #19383

Merged

Conversation

stevetracvc
Copy link
Contributor

SUMMARY

Sometimes, the values in a table cell can be really long. This PR shortens long cells with an ellipsis, and then expands the full content when hovering over the cell. This setting uses the "min width" for a column as the max width, so it really just fully fixes the column width to this value.

BEFORE/AFTER

chrome-capture-2022-2-26
SCREENSHOTS OR ANIMATED GIF

ADDITIONAL INFORMATION

  • Has associated issue: Table Column Max Width #19382
  • Fixes issue: Table Column Max Width #19382
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@stevetracvc
Copy link
Contributor Author

This PR allows per-column control. I have another branch that is a table-wide setting, and then any column with a min-width setting would get truncated. I think this per-column method is probably better and more clear to the table designer.

@codecov
Copy link

codecov bot commented Mar 26, 2022

Codecov Report

Merging #19383 (4a236a8) into master (7e9b85f) will decrease coverage by 0.00%.
The diff coverage is 37.50%.

@@            Coverage Diff             @@
##           master   #19383      +/-   ##
==========================================
- Coverage   66.43%   66.42%   -0.01%     
==========================================
  Files        1721     1721              
  Lines       64571    64589      +18     
  Branches     6814     6826      +12     
==========================================
+ Hits        42895    42901       +6     
- Misses      19942    19953      +11     
- Partials     1734     1735       +1     
Flag Coverage Δ
javascript 51.29% <37.50%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
...frontend/plugins/plugin-chart-table/src/Styles.tsx 100.00% <ø> (ø)
...tend/plugins/plugin-chart-table/src/TableChart.tsx 37.90% <28.57%> (-0.87%) ⬇️
...trols/components/ColumnConfigControl/constants.tsx 100.00% <100.00%> (ø)
...et-frontend/src/components/EditableTitle/index.tsx 64.28% <0.00%> (-5.72%) ⬇️
.../components/Header/HeaderActionsDropdown/index.jsx 68.33% <0.00%> (-3.34%) ⬇️
...ponents/ReportModal/HeaderReportDropdown/index.tsx 68.00% <0.00%> (-1.34%) ⬇️
.../explore/components/ExploreViewContainer/index.jsx 52.02% <0.00%> (-0.55%) ⬇️
superset-frontend/src/components/Icons/index.tsx 100.00% <0.00%> (ø)
...end/src/components/PageHeaderWithActions/index.tsx 90.00% <0.00%> (ø)
...perset-ui-core/src/dimension/computeMaxFontSize.ts 100.00% <0.00%> (ø)
... and 2 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 7e9b85f...4a236a8. Read the comment docs.

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

Nice feature!

@stevetracvc
Copy link
Contributor Author

@ktmud I need to look at the conflicts with the current master. Also I think I missed something

@stephenLYZ
Copy link
Member

@stevetracvc Great Work! Not a related issue. It looks like the description label here was placed below when hovering the button.
image

@rusackas rusackas merged commit 7e504ff into apache:master Jul 7, 2022
@stevetracvc
Copy link
Contributor Author

@stevetracvc Great Work! Not a related issue. It looks like the description label here was placed below when hovering the button. image

Thanks! And maybe I can set width on that control...

akshatsri pushed a commit to charan1314/superset that referenced this pull request Jul 19, 2022
…#19383)

* feat: truncate long values, a per-column setting

* fix: lint

* fix: removed width for column control

* fix: removed truncate option for time, bool, and numeric columns

* prevent extra div if not truncating
@kotanyi
Copy link

kotanyi commented Oct 20, 2022

@rusackas @ktmud @stephenLYZ

Hi,

I was just wondering if there's any plans for this to be included in the 2.0.1. release?
If not, any idea which release it might be included in?
(And if there's a place where I can find information like this, linking me to it will be greatly appreciated :) )

Thank you very much for any pointers!

Martin

@jaroet
Copy link

jaroet commented Mar 7, 2024

I had a columns that was very wide but has no spaces in it. Like 200 short words with a , in between the words but nowhere a space. The truncate does not work in that case. In excel this does work. It just truncates on some default value when it can't fine any spaces I guess.

@rusackas
Copy link
Member

rusackas commented Mar 7, 2024

I had a columns that was very wide but has no spaces in it. Like 200 short words with a , in between the words but nowhere a space. The truncate does not work in that case. In excel this does work. It just truncates on some default value when it can't fine any spaces I guess.

Hmmm... can't see why it doesn't work. If you can make a reproducible test case that we can all try (using example data or a trumped up SQL query) then you can file that on a new issue and hopefully someone can take a look. We're also open to PRs to dial things in, if you already see a root cause.

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants