Re-add (unrevert) "Replace ActionBar overflow calculations with CSS wrapping approach"

.changeset/many-suns-promise.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+Replace `ActionBar` overflow calculations with CSS wrapping approach to improve performance and stability

e2e/components/drafts/ActionBar.test.ts

@@ -47,12 +47,12 @@ test.describe('ActionBar', () => {
             },
           })
           const toolbarButtonSelector = `button[data-component="IconButton"]`
-          await expect(page.locator(toolbarButtonSelector)).toHaveCount(10)
+          await expect(page.locator(toolbarButtonSelector).filter({visible: true})).toHaveCount(10)
           await page.setViewportSize({width: viewports['primer.breakpoint.xs'], height: 768})
           await page.getByLabel('Task List').waitFor({
             state: 'hidden',
           })
-          await expect(page.locator(toolbarButtonSelector)).toHaveCount(8)
+          await expect(page.locator(toolbarButtonSelector).filter({visible: true})).toHaveCount(8)
           const moreButtonSelector = page.getByLabel('More Comment box toolbar items')
           await moreButtonSelector.click()
           await expect(page.locator('ul[role="menu"] [role="menuitem"]')).toHaveCount(3)

packages/react/src/ActionBar/ActionBar.examples.stories.module.css

@@ -12,23 +12,32 @@
 
 .CommentBoxHeader {
   display: flex;
-  flex-direction: row;
+  gap: var(--base-size-16);
   background-color: var(--bgColor-muted);
   border-top-left-radius: var(--borderRadius-medium);
   border-top-right-radius: var(--borderRadius-medium);
-  justify-content: space-between;
 }
 
-.CommentBoxHeaderLeft {
-  width: 100%;
-  min-width: 0;
+.CommentBoxHeaderViewSwitch {
+  flex-basis: 0;
+  display: flex;
+  height: 37px;
 }
 
-.CommentBoxHeaderRight {
-  display: flex;
-  gap: var(--base-size-8);
+.CommentBoxHeaderToolbar {
+  flex: 1;
   justify-content: flex-end;
   align-items: center;
+  min-width: 0;
+  display: flex;
+  padding-right: var(--base-size-4);
+
+  .CommentBoxHeaderActionBar {
+    flex: 1;
+    justify-content: flex-end;
+    padding-inline: 0;
+    overflow: hidden;
+  }
 }
 
 .DialogContent {
@@ -69,3 +78,7 @@
 .MultipleActionBarsSection {
   padding: var(--base-size-16);
 }
+
+.OverflowHidden {
+  overflow: hidden;
+}

packages/react/src/ActionBar/ActionBar.examples.stories.tsx

@@ -56,14 +56,6 @@ export const WithGroups = () => (
   </ActionBar>
 )
 
-export const TextLabels = () => (
-  <ActionBar aria-label="Toolbar">
-    <Button>Edit</Button>
-    <Button>Duplicate</Button>
-    <Button>Export to CSV</Button>
-  </ActionBar>
-)
-
 export const SmallActionBar = () => (
   <ActionBar size="small" aria-label="Toolbar">
     <ActionBar.IconButton icon={BoldIcon} aria-label="Bold"></ActionBar.IconButton>
@@ -128,8 +120,13 @@ export const CommentBox = (props: CommentBoxProps) => {
   return (
     <div className={classes.CommentBoxContainer}>
       <header className={classes.CommentBoxHeader}>
-        <div className={classes.CommentBoxHeaderLeft}>
-          <ActionBar aria-label={toolBarLabel}>
+        <div className={classes.CommentBoxHeaderViewSwitch}>
+          <Button variant="invisible">Write</Button>
+          <Button variant="invisible">Preview</Button>
+        </div>
+
+        <div className={classes.CommentBoxHeaderToolbar}>
+          <ActionBar aria-label={toolBarLabel} className={classes.CommentBoxHeaderActionBar} gap="none">
             <ActionBar.IconButton icon={HeadingIcon} aria-label="Heading"></ActionBar.IconButton>
             <ActionBar.IconButton icon={BoldIcon} aria-label="Bold"></ActionBar.IconButton>
             <ActionBar.IconButton icon={ItalicIcon} aria-label="Italic"></ActionBar.IconButton>
@@ -148,10 +145,6 @@ export const CommentBox = (props: CommentBoxProps) => {
             ></ActionBar.IconButton>
           </ActionBar>
         </div>
-        <div className={classes.CommentBoxHeaderRight}>
-          <Button variant="invisible">Write</Button>
-          <Button variant="invisible">Preview</Button>
-        </div>
       </header>
       <Textarea value={value} onChange={e => setValue(e.target.value)} id="markdowninput" aria-label="Markdown value" />
       <Dialog aria-labelledby="header" returnFocusRef={buttonRef} isOpen={isOpen} onDismiss={() => setIsOpen(false)}>

packages/react/src/ActionBar/ActionBar.module.css

@@ -6,10 +6,35 @@
   /* wonder why this is here */
   /* stylelint-disable-next-line primer/spacing */
   margin-bottom: -1px;
-  white-space: nowrap;
   list-style: none;
-  align-items: center;
+  align-items: flex-start;
   gap: var(--actionbar-gap, var(--stack-gap-condensed));
+  overflow: hidden;
+  /* Explicit height is required to clip wrapped items */
+  height: var(--actionbar-height, var(--control-small-size));
+
+  /* Scroll-based animations have no effect unless the container is scrollable (has overflow, even with overflow:hidden)
+  so we can use them to detect overflow. It would be cleaner to use scroll-state container queries for this, but
+  browser support for scroll-driven animations is slightly better. */
+  animation: detect-overflow linear;
+  animation-timeline: scroll(self block);
+
+  /* After initial render, JS is used to control visibility which provides progressive enhancement for unsupported browsers */
+  &[data-has-overflow='true'] {
+    --morebutton-display: flex;
+  }
+
+  &:where([data-size='small']) {
+    --actionbar-height: var(--control-small-size);
+  }
+
+  &:where([data-size='medium']) {
+    --actionbar-height: var(--control-medium-size);
+  }
+
+  &:where([data-size='large']) {
+    --actionbar-height: var(--control-large-size);
+  }
 
   /* Gap scale (mirrors Stack) */
   &:where([data-gap='none']) {
@@ -23,6 +48,20 @@
   &:where([data-gap='condensed']) {
     --actionbar-gap: var(--stack-gap-condensed);
   }
+
+  & [data-overflowing] {
+    /* Hide overflowing items. Even though they are clipped by `overflow: hidden`, setting `visibility: hidden` ensures
+    they can't accidentally be shown and also hides them from screen readers / keyboard nav. `!important` prevents
+    consumers from unintentionally overriding this and breaking accessibility. */
+    visibility: hidden !important;
+  }
+}
+
+@keyframes detect-overflow {
+  0%,
+  100% {
+    --morebutton-display: flex;
+  }
 }
 
 .Nav {
@@ -44,10 +83,24 @@
     content: '';
     /* stylelint-disable-next-line primer/colors */
     background: var(--borderColor-muted);
+    /* stylelint-disable-next-line primer/spacing */
+    margin-top: calc((var(--actionbar-height) - var(--base-size-20)) / 2);
   }
 }
 
 .Group {
   display: flex;
   gap: inherit;
 }
+
+.OverflowContainer {
+  display: flex;
+  flex-wrap: wrap;
+  gap: inherit;
+  justify-content: flex-end;
+  overflow: hidden;
+}
+
+.MoreButton {
+  display: var(--morebutton-display, none);
+}

packages/react/src/ActionBar/ActionBar.test.tsx

@@ -228,13 +228,14 @@ describe('ActionBar Registry System', () => {
     render(
       <div style={{width: 0, overflow: 'hidden'}}>
         <ActionBar aria-label="Zero width">
-          <ActionBar.IconButton icon={BoldIcon} aria-label="Zero width button" />
+          <ActionBar.IconButton icon={BoldIcon} aria-label="Zero width button" data-testid="zero-width-button" />
         </ActionBar>
       </div>,
     )
 
     // Component should still render even with zero width
-    expect(screen.getByRole('button', {name: 'Zero width button'})).toBeInTheDocument()
+    // Button is unlabeled because the label is hidden, so we select by test id instead
+    expect(screen.getByTestId('zero-width-button')).toBeInTheDocument()
   })
 
   it('should clean up registry on unmount', async () => {