-
Notifications
You must be signed in to change notification settings - Fork 543
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(ActionList): do not truncate description by default #5169
Merged
francinelucca
merged 16 commits into
main
from
francinelucca/3472-prcactionlist-inline-actionlist-description-is-automatically-truncated
Nov 12, 2024
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
a26447e
fix(ActionList): do not truncate description by default
francinelucca 8e900aa
docs(Description): document new truncate prop
francinelucca 72d9d4a
Create cyan-lions-itch.md
francinelucca 1239c13
test(vrt): update snapshots
francinelucca 6765fb3
Merge branch 'main' of github.com:primer/react into francinelucca/347…
francinelucca 8cc03e1
fix(ActionList): only break word on label if no inline description
francinelucca f59b0cf
test(vrt): update snapshots
francinelucca 61c3a35
Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…
francinelucca 4d4f4b5
Merge branch 'main' of github.com:primer/react into francinelucca/347…
francinelucca 5b237f5
Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…
francinelucca 323db8c
Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…
francinelucca 9a78287
Merge branch 'main' of github.com:primer/react into francinelucca/347…
francinelucca c6c2ec5
test: reproduce overlay issue
francinelucca 713082b
Revert "test: reproduce overlay issue"
francinelucca 6e3b3ef
Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…
francinelucca 4081b89
Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…
francinelucca File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@primer/react": minor | ||
--- | ||
|
||
fix(ActionList): do not truncate description by default |
Binary file modified
BIN
+3.43 KB
(110%)
...test.ts-snapshots/ActionList-Text-Wrap-And-Truncation-dark-colorblind-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.39 KB
(110%)
...ist.test.ts-snapshots/ActionList-Text-Wrap-And-Truncation-dark-dimmed-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.55 KB
(110%)
...t.ts-snapshots/ActionList-Text-Wrap-And-Truncation-dark-high-contrast-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.43 KB
(110%)
...ActionList.test.ts-snapshots/ActionList-Text-Wrap-And-Truncation-dark-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.43 KB
(110%)
...test.ts-snapshots/ActionList-Text-Wrap-And-Truncation-dark-tritanopia-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.54 KB
(110%)
...est.ts-snapshots/ActionList-Text-Wrap-And-Truncation-light-colorblind-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.08 KB
(110%)
....ts-snapshots/ActionList-Text-Wrap-And-Truncation-light-high-contrast-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.54 KB
(110%)
...ctionList.test.ts-snapshots/ActionList-Text-Wrap-And-Truncation-light-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+3.54 KB
(110%)
...est.ts-snapshots/ActionList-Text-Wrap-And-Truncation-light-tritanopia-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,19 +14,24 @@ export type ActionListDescriptionProps = { | |
*/ | ||
variant?: 'inline' | 'block' | ||
className?: string | ||
/** | ||
* Whether the inline description should truncate the text on overflow. | ||
*/ | ||
truncate?: boolean | ||
} & SxProp | ||
|
||
export const Description: React.FC<React.PropsWithChildren<ActionListDescriptionProps>> = ({ | ||
variant = 'inline', | ||
sx = {}, | ||
className, | ||
truncate, | ||
...props | ||
}) => { | ||
const styles = { | ||
fontSize: 0, | ||
lineHeight: '16px', | ||
flexGrow: 1, | ||
flexBasis: 0, | ||
flexBasis: variant === 'inline' && !truncate ? 'auto' : 0, | ||
minWidth: 0, | ||
marginLeft: variant === 'block' ? 0 : 2, | ||
color: 'fg.muted', | ||
|
@@ -37,11 +42,11 @@ export const Description: React.FC<React.PropsWithChildren<ActionListDescription | |
|
||
const {blockDescriptionId, inlineDescriptionId} = React.useContext(ItemContext) | ||
|
||
return variant === 'block' ? ( | ||
return variant === 'block' || !truncate ? ( | ||
<Box | ||
as="span" | ||
sx={merge(styles, sx as SxProp)} | ||
id={blockDescriptionId} | ||
id={variant === 'block' ? blockDescriptionId : inlineDescriptionId} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't know we have different IDs for inline and block 💭 good to know There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was surprised by this one as well hahaah |
||
className={className} | ||
data-component="ActionList.Description" | ||
> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Very detail tests!! Love it 🔥
I’m also curious to learn when you prefer to use Jest for visual style testing versus a tool like Playwright. 👀
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.
That's a great question! I actually didn't take the fact that we already had a playwright test for the truncation story into account when writing these so this might be a bit "overkill" haha. I think I tend to go for Jest because it's what I'm most familiar with but love how "complete" vrt testing is in that it'll fail on the smallest visual change in the entire story so it feels like it covers more ground than a specific jest test. That being said I'm a big fan of redundancy.
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.
Love this!