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

Truncate long labels in x axis #6631

Merged
merged 1 commit into from Jan 14, 2019
Merged

Conversation

betodealmeida
Copy link
Member

The padding for NVD3 charts is computed based on the length of the longest label in axes. For datasets with really long labels this results in excessive padding.

I added code to truncate the labels that are longer than 24 characters. Note that this has to be done twice: first when computing the length of the longest label, and again after the chart has been redrawn.

An example with MAX_LABEL_LENGTH set to 8:

screen shot 2019-01-10 at 3 01 39 pm

@codecov-io
Copy link

codecov-io commented Jan 10, 2019

Codecov Report

Merging #6631 into master will decrease coverage by 0.01%.
The diff coverage is 14.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6631      +/-   ##
==========================================
- Coverage   56.24%   56.23%   -0.02%     
==========================================
  Files         518      518              
  Lines       23007    23014       +7     
  Branches     2750     2753       +3     
==========================================
+ Hits        12941    12942       +1     
- Misses       9655     9660       +5     
- Partials      411      412       +1
Impacted Files Coverage Δ
superset/assets/src/visualizations/nvd3/NVD3Vis.js 0% <0%> (ø) ⬆️
superset/assets/src/visualizations/nvd3/utils.js 12% <20%> (+0.33%) ⬆️

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 a2ce997...4033979. Read the comment docs.

@mistercrunch
Copy link
Member

I agree we need to truncate, though we may want to have a way for users to see the long string. I'm not sure but maybe <text title="{notTruncated}">{truncated}</text> would work out of the box in SVG. Otherwise a combo of this and d3tip maybe. All this ideally only where notTruncated !== truncated to avoid bloating the dom with dup info.

Also 24 seems like a decent choice, but is arbitrary. If we are to fully truncate the info and not have it available on hover, we may want to parameterize it...

@mistercrunch
Copy link
Member

Nevermind, didn't realized the full description is in the tooltip. LGTM

@betodealmeida betodealmeida merged commit 5055157 into apache:master Jan 14, 2019
betodealmeida added a commit to lyft/incubator-superset that referenced this pull request Jan 14, 2019
(cherry picked from commit 5055157)
mistercrunch added a commit to mistercrunch/superset that referenced this pull request Jan 18, 2019
mistercrunch added a commit that referenced this pull request Jan 19, 2019
@kristw kristw added enhancement:request Enhancement request submitted by anyone from the community and removed #refine labels Mar 20, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 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 enhancement:request Enhancement request submitted by anyone from the community v0.31 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants