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

Add vx text truncation #447

Closed
wants to merge 1 commit into from

Conversation

MikeyParton
Copy link

🚀 Enhancements

I have a use case where I needed to restrict some text labels to a maximum number of lines.

Adds lineClamp and truncateText props to vx/text.
lineClamp restricts the number of lines of the rendered text. When the lineClamp is exceeded, the text is truncated with the truncateText which by default is '...'

Here's an example:
https://codesandbox.io/s/3v3rn68mj6

@zmitry
Copy link

zmitry commented Apr 16, 2019

Great

@joshnabbott
Copy link

I could use this feature.

@@ -149,6 +170,7 @@ Text.defaultProps = {
capHeight: '0.71em', // Magic number from d3
scaleToFit: false,
textAnchor: 'start',
truncateText: '...',
Copy link

Choose a reason for hiding this comment

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

I'd suggest using … symbol

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree

@williaster
Copy link
Collaborator

Thanks for the contribution, I'm sorry it wasn't reviewed and merged within any reasonable time frame.

Since the repo has been re-written in TS, it's not super easy to merge this anymore. I'm committed to reviewing it / getting it in if you are interested in rebasing on the latest master 🙏

@MikeyParton
Copy link
Author

All good. I just found there are some extra edge cases that this doesn't account for either e.g. accounting for the length of interpreted html character codes. I don't really need this feature anymore, so I'm happy to close it for now - it seems more trouble than it's worth.

@dimitrisnl
Copy link

Is there any interest to reimplement this feature?

@williaster
Copy link
Collaborator

Still happy to review a PR on my end

@MikeyParton
Copy link
Author

Personally I don't need this feature anymore and I don't really have time to reimplement it at the moment.

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.

None yet

5 participants