Skip to content

feat(Truncate): added logic to truncate based on max characters #11742

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 21, 2025
Merged

feat(Truncate): added logic to truncate based on max characters #11742

merged 5 commits into from
Apr 21, 2025

Conversation

thatblindgeye
Copy link
Contributor

What: Closes #10892

Additional issues:

@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 4, 2025 •
edited
Loading

Preview: https://patternfly-react-pr-11742.surge.sh

A11y report: https://patternfly-react-pr-11742-a11y.surge.sh

@thatblindgeye thatblindgeye mentioned this pull request Apr 4, 2025
Closed
@thatblindgeye thatblindgeye force-pushed the iss10892_trueMidTrunc branch from ffe6ebb to 656a30c Compare April 11, 2025 12:49
@thatblindgeye thatblindgeye marked this pull request as ready for review April 11, 2025 12:49
@thatblindgeye thatblindgeye requested review from mcoker, a team, rebeccaalpert and wise-king-sullyman and removed request for a team April 11, 2025 12:49
mcoker
mcoker approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM! Just a comment, but "maxCharsDisplayed" kinda sounds like it might show less characters, but will only show up to whatever the val of maxCharsDisplayed is. As in it would pair with something like minCharsDisplayed. When this prop as it is now is more just charsDisplayed. FWIW, core also has --MinWidth vars used in the truncate component that could be modified by users via a minCharsDisplayed prop.

I see how it also sounds appropriate - it will only show characters up to whatever value is passed.

Not saying anything needs to change, but wanted to share in case anyone else felt similarly.

@thatblindgeye thatblindgeye force-pushed the iss10892_trueMidTrunc branch from 656a30c to c609395 Compare April 17, 2025 15:19
@thatblindgeye thatblindgeye requested a review from kmcfaul April 17, 2025 15:19
rebeccaalpert
rebeccaalpert approved these changes Apr 17, 2025
View reviewed changes
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

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

This looked good to me!

mcoker
mcoker approved these changes Apr 17, 2025
View reviewed changes
Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

This PR is great!
I already approved, but
It's haiku day! Yay!

@kmcfaul kmcfaul merged commit b7456f9 into patternfly:main Apr 21, 2025
13 checks passed
@thatblindgeye thatblindgeye mentioned this pull request Apr 21, 2025
Closed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rebeccaalpert rebeccaalpert rebeccaalpert approved these changes

@mcoker mcoker mcoker approved these changes

@kmcfaul kmcfaul kmcfaul approved these changes

@wise-king-sullyman wise-king-sullyman Awaiting requested review from wise-king-sullyman wise-king-sullyman was automatically assigned from patternfly/react-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Truncate - True calculated middle truncate
5 participants
@thatblindgeye @patternfly-build @rebeccaalpert @mcoker @kmcfaul