fix(ActionList): do not truncate description by default #5169

francinelucca · 2024-10-23T03:46:54Z

Closes https://github.com/github/primer/issues/3472, Closes #4664

The current react implementation of ActionList.Description truncates the text by default which poses accessibility concerns due to the content not being able to be read by keyboard users. Additionally, the inline truncation can cause horizontal overflow which causes a WCAG reflow problem. This PR mimics the functionality of PVC's ActionList, which wraps the text instead of truncating by default. An additional truncate prop has been introduced to allow truncation functionality when applicable (documented here)

Before

After

Changelog

New

Tests for ActionList.Description in block, inline truncation and inline no-truncation variants
New truncate prop on ActionList.Description

Changed

Modify ActionList TextWrapAndTruncation story to include new truncation/no-truncation use cases
Modify render behavior of ActionList.Description to not render inside a Truncate component if new truncate prop is false (false by default)
flex-basis style change on ActionList.Description to grow automatically if truncate is false
word-break is now set to normal instead of break-word if the Actionlist.Item contains an inline Actionlist.Description
Updates snapshots for VRT tests

Rollout strategy

Patch release
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Testing & Reviewing

Test new truncation behavior in TextWrapAndTruncation story

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

changeset-bot · 2024-10-23T03:46:58Z

🦋 Changeset detected

Latest commit: 6e3b3ef

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-10-23T03:47:29Z

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

github-actions · 2024-10-23T03:50:03Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	98.73 KB (+0.07% 🔺)
packages/react/dist/browser.umd.js	99.07 KB (0%)

primer-integration · 2024-10-23T17:07:58Z

🟢 golden-jobs completed with status success.

francinelucca · 2024-10-23T22:20:46Z

Hi @langermank. Wanted to checkin about a slight difference I'm seeing here between PVC/PRC in relations to this PR and the word-break property. Wondering if we're ok with this or if we want to rethink this change? See video below

Screen.Recording.2024-10-23.at.6.15.04.PM.mov

…2-prcactionlist-inline-actionlist-description-is-automatically-truncated

This reverts commit c6c2ec5.

…onlist-description-is-automatically-truncated

primer-integration · 2024-11-06T21:26:15Z

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/350286

francinelucca · 2024-11-08T04:15:46Z

Will need these changes to be pulled into dotcom upon PRC version update

langermank

Lets go! 🚀

broccolinisoup

so cool 🔥

broccolinisoup · 2024-11-12T06:53:55Z

packages/react/src/ActionList/Description.tsx

    <Box
      as="span"
      sx={merge(styles, sx as SxProp)}
-      id={blockDescriptionId}
+      id={variant === 'block' ? blockDescriptionId : inlineDescriptionId}


I didn't know we have different IDs for inline and block 💭 good to know

was surprised by this one as well hahaah

broccolinisoup · 2024-11-12T06:58:35Z

packages/react/src/ActionList/ActionList.test.tsx

+      expect(description).toHaveStyleRule('flex-basis', '0')
+      expect(description).toHaveStyleRule('text-overflow', 'ellipsis')
+      expect(description).toHaveStyleRule('overflow', 'hidden')
+      expect(description).toHaveStyleRule('white-space', 'nowrap')


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. 👀

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.

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.

Love this!

…onlist-description-is-automatically-truncated

francinelucca · 2024-11-12T20:01:15Z

Will need these changes to be pulled into dotcom upon PRC version update

Tagging release conductor as FYI @siddharthkp, @broccolinisoup since I see you're next :)

francinelucca added 2 commits October 22, 2024 23:28

fix(ActionList): do not truncate description by default

a26447e

docs(Description): document new truncate prop

8e900aa

francinelucca added status: wip component: ActionList staff Author is a staff member labels Oct 23, 2024

github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Oct 23, 2024

Create cyan-lions-itch.md

72d9d4a

francinelucca temporarily deployed to github-pages October 23, 2024 03:50 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-5169 October 23, 2024 03:51 Inactive

francinelucca added the update snapshots label Oct 23, 2024

test(vrt): update snapshots

1239c13

github-actions bot removed the update snapshots label Oct 23, 2024

primer bot temporarily deployed to github-pages October 23, 2024 15:13 Inactive

github-actions bot temporarily deployed to storybook-preview-5169 October 23, 2024 15:13 Inactive

francinelucca requested a review from langermank October 23, 2024 22:20

Merge branch 'main' of github.com:primer/react into francinelucca/347…

6765fb3

…2-prcactionlist-inline-actionlist-description-is-automatically-truncated

francinelucca temporarily deployed to github-pages October 25, 2024 20:17 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-5169 October 25, 2024 20:18 Inactive

fix(ActionList): only break word on label if no inline description

8cc03e1

francinelucca temporarily deployed to github-pages October 25, 2024 20:37 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-5169 October 25, 2024 20:38 Inactive

francinelucca added the update snapshots label Oct 25, 2024

test(vrt): update snapshots

f59b0cf

github-actions bot removed the update snapshots label Oct 25, 2024

primer bot temporarily deployed to github-pages October 25, 2024 20:53 Inactive

francinelucca temporarily deployed to github-pages November 6, 2024 20:11 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-5169 November 6, 2024 20:11 Inactive

francinelucca and others added 2 commits November 6, 2024 15:46

Revert "test: reproduce overlay issue"

713082b

This reverts commit c6c2ec5.

Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…

6e3b3ef

…onlist-description-is-automatically-truncated

francinelucca temporarily deployed to github-pages November 6, 2024 20:48 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-5169 November 6, 2024 20:48 Inactive

francinelucca added status: review needed and removed status: wip labels Nov 8, 2024

francinelucca marked this pull request as ready for review November 8, 2024 04:16

francinelucca requested review from a team as code owners November 8, 2024 04:16

francinelucca requested review from broccolinisoup and TylerJDev November 8, 2024 04:16

langermank approved these changes Nov 8, 2024

View reviewed changes

broccolinisoup approved these changes Nov 12, 2024

View reviewed changes

Merge branch 'main' into francinelucca/3472-prcactionlist-inline-acti…

4081b89

…onlist-description-is-automatically-truncated

francinelucca deployed to github-pages November 12, 2024 17:20 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-5169 November 12, 2024 17:21 View deployment

francinelucca added this pull request to the merge queue Nov 12, 2024

Merged via the queue into main with commit a2efba0 Nov 12, 2024
43 checks passed

francinelucca deleted the francinelucca/3472-prcactionlist-inline-actionlist-description-is-automatically-truncated branch November 12, 2024 20:12

primer bot mentioned this pull request Nov 12, 2024

Release tracking (37.5.0) #5265

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ActionList): do not truncate description by default #5169

fix(ActionList): do not truncate description by default #5169

francinelucca commented Oct 23, 2024 •

edited

Loading

changeset-bot bot commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 23, 2024 •

edited

Loading

primer-integration bot commented Oct 23, 2024 •

edited

Loading

francinelucca commented Oct 23, 2024

primer-integration bot commented Nov 6, 2024

francinelucca commented Nov 8, 2024

langermank left a comment

broccolinisoup left a comment

broccolinisoup Nov 12, 2024

francinelucca Nov 12, 2024

broccolinisoup Nov 12, 2024

francinelucca Nov 12, 2024

broccolinisoup Nov 12, 2024

francinelucca commented Nov 12, 2024

fix(ActionList): do not truncate description by default #5169

fix(ActionList): do not truncate description by default #5169

Conversation

francinelucca commented Oct 23, 2024 • edited Loading

Changelog

New

Changed

Rollout strategy

Testing & Reviewing

Merge checklist

changeset-bot bot commented Oct 23, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 23, 2024 • edited Loading

size-limit report 📦

primer-integration bot commented Oct 23, 2024 • edited Loading

francinelucca commented Oct 23, 2024

primer-integration bot commented Nov 6, 2024

francinelucca commented Nov 8, 2024

langermank left a comment

Choose a reason for hiding this comment

broccolinisoup left a comment

Choose a reason for hiding this comment

broccolinisoup Nov 12, 2024

Choose a reason for hiding this comment

francinelucca Nov 12, 2024

Choose a reason for hiding this comment

broccolinisoup Nov 12, 2024

Choose a reason for hiding this comment

francinelucca Nov 12, 2024

Choose a reason for hiding this comment

broccolinisoup Nov 12, 2024

Choose a reason for hiding this comment

francinelucca commented Nov 12, 2024

francinelucca commented Oct 23, 2024 •

edited

Loading

changeset-bot bot commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024 •

edited

Loading

primer-integration bot commented Oct 23, 2024 •

edited

Loading