-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix(ui-truncate): truncate should not split words appart #690
fix(ui-truncate): truncate should not split words appart #690
Conversation
WalkthroughThe changes in this pull request involve a modification of the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Bundle Size (components)
Overall bundle size: 84.29 KB (+13 B +0.02%) Bundle Size (fingerprint)
Overall bundle size: 48.28 KB (-1 B 0.00%) Bundle Size (form components)
Overall bundle size: 49.9 KB (+3 B +0.01%) Bundle Size (system)
Overall bundle size: 50.35 KB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
packages/ui-truncate/src/components/Truncate/utilities.ts (1)
12-36
: LGTM: Improved truncation logicThe new implementation of the
truncate
function aligns well with its description and the PR objectives. It correctly handles different scenarios, such as when theidealLength
ends on a space or in the middle of a word, ensuring that words are not split apart during truncation.The removal of the omission logic simplifies the function and makes it more focused on its primary task. The overall logic is clear and concise.
Consider renaming the
nextSpace
variable tonextSpaceIndex
for better clarity:- const nextSpace = string.slice(idealLength).search(" "); + const nextSpaceIndex = string.slice(idealLength).search(" "); return { - string: string.slice(0, idealLength + nextSpace), + string: string.slice(0, idealLength + nextSpaceIndex), isTruncated: true, };packages/ui-truncate/src/components/Truncate/__tests__/utilities.test.ts (2)
13-19
: Consider differentiating this test case from the previous one.This test case has the same expected output as the previous one, despite having a different
idealLength
. To make the test suite more robust and clear:
- Consider modifying the input string or expected output to better demonstrate the "truncate at the next space found right after the ideal length" behavior.
- Update the test case name to more accurately reflect the specific scenario being tested.
Here's a suggested modification:
it("should truncate at the next space after the ideal length", () => { const result = truncate({ string: "Hello there, World!", idealLength: 7, }); expect(result).toEqual({ string: "Hello there,", isTruncated: true }); });This change would better illustrate the difference between truncating at the ideal length and truncating at the next space after the ideal length.
21-27
: Approve implementation, but suggest renaming the test case.The test case correctly demonstrates the truncation behavior with the new
idealLength
parameter. However, to improve clarity and differentiate it from the previous test case:Consider renaming this test case to better reflect its specific scenario. For example:
it("should truncate a longer string at the next space after the ideal length", () => { // ... (rest of the test case remains the same) });This name more accurately describes what this particular test case is verifying.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- packages/ui-truncate/src/components/Truncate/Truncate.tsx (1 hunks)
- packages/ui-truncate/src/components/Truncate/tests/Truncate.test.tsx (1 hunks)
- packages/ui-truncate/src/components/Truncate/tests/utilities.test.ts (1 hunks)
- packages/ui-truncate/src/components/Truncate/utilities.ts (1 hunks)
Additional comments not posted (7)
packages/ui-truncate/src/components/Truncate/Truncate.tsx (1)
20-20
: Approve the parameter name change and suggest utility function verification.The change from
length
toidealLength: length
aligns well with the PR objective of not splitting words apart during truncation. This semantic shift suggests a more flexible approach to determining the truncation point, which should lead to improved readability of the truncated text.To ensure full compatibility, please verify that the
truncate
utility function in./utilities.ts
has been updated to handle thisidealLength
parameter correctly. Run the following script to check the implementation:If the script doesn't return any results, it might indicate that the
truncate
function hasn't been updated to match this new parameter name. In that case, please update the utility function accordingly.packages/ui-truncate/src/components/Truncate/utilities.ts (2)
5-5
: LGTM: Improved type definitionThe change from
length
toidealLength
in theTruncateOptions
type better reflects the function's behavior. Removing theomission
property simplifies the API, which is a good improvement. MakingidealLength
optional allows for using a default value, providing flexibility to the users of this function.
12-36
: Great job addressing the PR objectiveThe changes in the
truncate
function directly address the PR objective of ensuring that words are not split apart during truncation. The new implementation looks for the next space after the ideal length if truncation is necessary, which guarantees that whole words are preserved. This is a significant improvement that enhances the usability and readability of the truncated text.packages/ui-truncate/src/components/Truncate/__tests__/utilities.test.ts (3)
5-11
: LGTM: Test case correctly uses the newidealLength
parameter.The test case has been updated to use
idealLength
instead oflength
, which aligns with the changes made to thetruncate
function. The expected result is appropriate for the given input and ideal length.
29-35
: LGTM: Edge case correctly handled.This test case effectively verifies the behavior when the
idealLength
is greater than the string length. It correctly expects the original string to be returned unchanged withisTruncated
set to false.
1-36
: Overall, the test cases have been successfully updated to reflect the new truncation behavior.The changes in this file align well with the PR objectives. All test cases now use
idealLength
instead oflength
, and they cover various scenarios including truncation at ideal length, truncation at the next space, and handling of edge cases where the ideal length exceeds the string length.To further improve the test suite:
- Consider implementing the suggested changes to differentiate the second and third test cases more clearly.
- Ensure that the test cases cover all possible edge cases and scenarios for the updated truncation logic.
To ensure comprehensive test coverage, let's verify if there are any other test files for the
truncate
function:This will help us confirm if we've reviewed all relevant test files for the
truncate
function.packages/ui-truncate/src/components/Truncate/__tests__/Truncate.test.tsx (1)
21-21
: Approved change, but clarification needed on truncation behaviorThe updated test expectation aligns with the PR objective of not splitting words apart during truncation. This is a positive change that should improve the user experience.
However, I have a few points that need clarification:
The test still uses the
length
prop with a value of 20, but the AI summary mentioned a change fromlength
toidealLength
. Can you confirm if this prop name change has been implemented in the component?The expected truncated text is now longer than the specified length of 20. Could you explain how the new truncation logic works? Is it now aiming for the nearest word boundary after the specified length?
Has the component's behavior changed, or was this test updated to match the existing behavior?
To help clarify these points, could you run the following script to check the implementation of the Truncate component?
🚀 Automated Release --- <details><summary>ui-anchor: 1.1.4</summary> ## [1.1.4](ui-anchor-v1.1.3...ui-anchor-v1.1.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.4 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-bubble: 1.0.4</summary> ## [1.0.4](ui-bubble-v1.0.3...ui-bubble-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-button: 1.1.4</summary> ## [1.1.4](ui-button-v1.1.3...ui-button-v1.1.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-icons bumped to 1.12.5 * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-card: 1.0.4</summary> ## [1.0.4](ui-card-v1.0.3...ui-card-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-components: 5.31.5</summary> ## [5.31.5](ui-components-v5.31.4...ui-components-v5.31.5) (2024-09-24) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-anchor bumped to 1.1.4 * @versini/ui-button bumped to 1.1.4 * @versini/ui-bubble bumped to 1.0.4 * @versini/ui-card bumped to 1.0.4 * @versini/ui-footer bumped to 1.0.4 * @versini/ui-header bumped to 1.0.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-main bumped to 1.0.4 * @versini/ui-menu bumped to 1.0.4 * @versini/ui-panel bumped to 1.0.4 * @versini/ui-pill bumped to 1.0.4 * @versini/ui-private bumped to 1.4.13 * @versini/ui-spinner bumped to 1.0.4 * @versini/ui-table bumped to 1.0.4 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-footer: 1.0.4</summary> ## [1.0.4](ui-footer-v1.0.3...ui-footer-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-form: 1.6.4</summary> ## [1.6.4](ui-form-v1.6.3...ui-form-v1.6.4) (2024-09-24) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-textarea bumped to 1.0.3 * @versini/ui-textinput bumped to 1.0.3 * @versini/ui-toggle bumped to 1.0.3 * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-header: 1.0.4</summary> ## [1.0.4](ui-header-v1.0.3...ui-header-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-icons: 1.12.5</summary> ## [1.12.5](ui-icons-v1.12.4...ui-icons-v1.12.5) (2024-09-24) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-private bumped to 1.4.13 </details> <details><summary>ui-main: 1.0.4</summary> ## [1.0.4](ui-main-v1.0.3...ui-main-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-menu: 1.0.4</summary> ## [1.0.4](ui-menu-v1.0.3...ui-menu-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-panel: 1.0.4</summary> ## [1.0.4](ui-panel-v1.0.3...ui-panel-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-pill: 1.0.4</summary> ## [1.0.4](ui-pill-v1.0.3...ui-pill-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-private: 1.4.13</summary> ## [1.4.13](ui-private-v1.4.12...ui-private-v1.4.13) (2024-09-24) ### Dependencies * The following workspace dependencies were updated * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-spinner: 1.0.4</summary> ## [1.0.4](ui-spinner-v1.0.3...ui-spinner-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-styles: 1.10.1</summary> ## [1.10.1](ui-styles-v1.10.0...ui-styles-v1.10.1) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) </details> <details><summary>ui-system: 1.4.14</summary> ## [1.4.14](ui-system-v1.4.13...ui-system-v1.4.14) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-table: 1.0.4</summary> ## [1.0.4](ui-table-v1.0.3...ui-table-v1.0.4) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-textarea: 1.0.3</summary> ## [1.0.3](ui-textarea-v1.0.2...ui-textarea-v1.0.3) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-textinput: 1.0.3</summary> ## [1.0.3](ui-textinput-v1.0.2...ui-textinput-v1.0.3) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-button bumped to 1.1.4 * @versini/ui-icons bumped to 1.12.5 * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-toggle: 1.0.3</summary> ## [1.0.3](ui-toggle-v1.0.2...ui-toggle-v1.0.3) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.13 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> <details><summary>ui-truncate: 1.0.1</summary> ## [1.0.1](ui-truncate-v1.0.0...ui-truncate-v1.0.1) (2024-09-24) ### Bug Fixes * bump non-breaking dependencies to latest ([#692](#692)) ([2300b7c](2300b7c)) * **ui-truncate:** truncate should not split words appart ([#690](#690)) ([67b1686](67b1686)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.4 * devDependencies * @versini/ui-styles bumped to 1.10.1 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: aversini <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Truncate
component's functionality by refining parameter names and logic.