diff --git a/.changeset/nav-list-item-gaps.md b/.changeset/nav-list-item-gaps.md new file mode 100644 index 00000000000..f9ea9996323 --- /dev/null +++ b/.changeset/nav-list-item-gaps.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +NavList: Graduate item gaps to the default experience. diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx index 9c9af6074a4..3e2536d9e9c 100644 --- a/packages/react/src/ActionList/ActionList.test.tsx +++ b/packages/react/src/ActionList/ActionList.test.tsx @@ -593,19 +593,6 @@ describe('ActionList with role="tree"', () => { expect(container.querySelector('[data-component="ActionList"]')).not.toHaveAttribute('data-item-gap') }) - - it('does not set data-item-gap on a standalone ActionList even when the primer_react_action_list_item_gap feature flag is enabled', () => { - const {container} = HTMLRender( - - - Home - About - - , - ) - - expect(container.querySelector('[data-component="ActionList"]')).not.toHaveAttribute('data-item-gap') - }) }) describe('ActionList forwarded ref (primer_react_merged_forwarded_refs)', () => { diff --git a/packages/react/src/ActionList/List.tsx b/packages/react/src/ActionList/List.tsx index d00ba6a95be..4b59ad9a1c5 100644 --- a/packages/react/src/ActionList/List.tsx +++ b/packages/react/src/ActionList/List.tsx @@ -51,7 +51,7 @@ const UnwrappedList = ( const providedOrCreatedRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const readRef = mergedRefEnabled ? listRef : providedOrCreatedRef const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef - const itemGapEnabled = useFeatureFlag('primer_react_action_list_item_gap') && container === 'NavList' + const itemGapEnabled = container === 'NavList' let enableFocusZone = false if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index dcf90e27d15..3add2b1cc70 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -6,7 +6,6 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({ primer_react_select_panel_order_selected_at_top: false, primer_react_styled_react_use_primer_theme_providers: false, primer_react_action_list_group_heading_trailing_action: false, - primer_react_action_list_item_gap: false, primer_react_timeline_list_semantics: false, primer_react_merged_forwarded_refs: false, primer_react_underline_panels_controlled: false, diff --git a/packages/react/src/NavList/NavList.features.stories.tsx b/packages/react/src/NavList/NavList.features.stories.tsx index b9488ea1fae..7168d96198c 100644 --- a/packages/react/src/NavList/NavList.features.stories.tsx +++ b/packages/react/src/NavList/NavList.features.stories.tsx @@ -22,7 +22,6 @@ import { import Octicon from '../Octicon' import VisuallyHidden from '../_VisuallyHidden' import {ReactRouterLikeLink} from '../Pagination/mocks/ReactRouterLink' -import {FeatureFlags} from '../FeatureFlags' const meta: Meta = { title: 'Components/NavList/Features', @@ -622,29 +621,27 @@ export const WithDescription: StoryFn = () => ( ) -export const WithItemGap: StoryFn = () => ( - - - - - - Home - - - About - - Team - History - - - Contact - - - - +export const WithGapBetweenItems: StoryFn = () => ( + + + + + Home + + + About + + Team + History + + + Contact + + + ) -WithItemGap.storyName = 'With gap between items (behind feature flag)' +WithGapBetweenItems.storyName = 'With gap between items' export const WithoutDivider: StoryFn = () => ( diff --git a/packages/react/src/NavList/NavList.test.tsx b/packages/react/src/NavList/NavList.test.tsx index 6fd762ea3f4..46148683195 100644 --- a/packages/react/src/NavList/NavList.test.tsx +++ b/packages/react/src/NavList/NavList.test.tsx @@ -5,7 +5,6 @@ import {renderToStaticMarkup} from 'react-dom/server' import {NavList, type NavListGroupHeadingProps} from './NavList' import {ReactRouterLikeLink} from '../Pagination/mocks/ReactRouterLink' import {implementsClassName} from '../utils/testing' -import {FeatureFlags} from '../FeatureFlags' import {asSlot} from '../utils/as-slot' type NextJSLinkProps = {href: string; children: React.ReactNode} @@ -601,8 +600,8 @@ describe('NavList.ShowMoreItem with pages', () => { expect(queryByRole('link', {name: 'Item 4'})).toHaveAttribute('href', '#item4') }) - describe('item gap feature flag', () => { - it('does not set data-item-gap on the underlying ActionList by default', () => { + describe('item gaps', () => { + it('sets data-item-gap on the underlying ActionList by default', () => { const {container} = render( @@ -612,44 +611,10 @@ describe('NavList.ShowMoreItem with pages', () => { , ) - expect(container.querySelector('[data-component="ActionList"]')).not.toHaveAttribute('data-item-gap') - }) - - it('sets data-item-gap on the underlying ActionList when the primer_react_action_list_item_gap feature flag is enabled', () => { - const {container} = render( - - - - Home - - About - - , - ) - expect(container.querySelector('[data-component="ActionList"]')).toHaveAttribute('data-item-gap', '') }) - it('adds a gap between a parent item and the first item of its expanded sub-nav when the feature flag is enabled', () => { - const {container} = render( - - - - Item 1 - - Sub item 1 - - - - , - ) - - const subGroup = container.querySelector('ul[aria-labelledby]') - expect(subGroup).not.toBeNull() - expect(getComputedStyle(subGroup as HTMLElement).marginBlockStart).toBe('2px') - }) - - it('does not add a gap before a sub-nav when the feature flag is disabled', () => { + it('adds a gap between a parent item and the first item of its expanded sub-nav by default', () => { const {container} = render( @@ -663,7 +628,7 @@ describe('NavList.ShowMoreItem with pages', () => { const subGroup = container.querySelector('ul[aria-labelledby]') expect(subGroup).not.toBeNull() - expect(getComputedStyle(subGroup as HTMLElement).marginBlockStart).toBe('0px') + expect(getComputedStyle(subGroup as HTMLElement).marginBlockStart).toBe('2px') }) })