Skip to content

Commit

Permalink
Revert "Pre-ActionList CSS Modules bug fixes (#5358)"
Browse files Browse the repository at this point in the history
This reverts commit b08f770.
  • Loading branch information
francinelucca committed Dec 5, 2024
1 parent b08f770 commit a0ebbde
Show file tree
Hide file tree
Showing 161 changed files with 177 additions and 209 deletions.
5 changes: 0 additions & 5 deletions .changeset/healthy-foxes-hunt.md

This file was deleted.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
28 changes: 28 additions & 0 deletions e2e/components/ActionList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,34 @@ test.describe('ActionList', () => {
}
})

test.describe('With Trailing Action', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-actionlist-features--with-trailing-action',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`ActionList.With Trailing Action.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-actionlist-features--with-trailing-action',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('Full Variant', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
30 changes: 30 additions & 0 deletions e2e/components/NavList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,34 @@ test.describe('NavList', () => {
})
}
})

test.describe('With Bad Example of SubNav and TrailingAction', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist-devonly--with-bad-example-of-sub-nav-and-trailing-action',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(
`NavList.With Bad Example of SubNav and TrailingAction.${theme}.png`,
)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist-devonly--with-bad-example-of-sub-nav-and-trailing-action',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})
})
206 changes: 114 additions & 92 deletions packages/react/src/ActionList/ActionList.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
GitPullRequestIcon,
IssueOpenedIcon,
ProjectIcon,
LinkExternalIcon,
} from '@primer/octicons-react'
import {FeatureFlags} from '../FeatureFlags'

Expand Down Expand Up @@ -280,7 +281,6 @@ export const InactiveSingleSelect = () => {
const [selectedIndex, setSelectedIndex] = React.useState(1)
return (
<ActionList selectionVariant="single" showDividers role="menu" aria-label="Project">
{/* menuitem because state is inactive */}
<ActionList.Item role="menuitem" selected={false} inactiveText="Unavailable due to an outage">
Inactive item
</ActionList.Item>
Expand Down Expand Up @@ -409,7 +409,6 @@ export const InactiveMultiselect = () => {
}
return (
<ActionList selectionVariant="multiple" role="menu" aria-label="Project">
{/* menuitem because state is inactive */}
<ActionList.Item role="menuitem" selected={false} inactiveText="Unavailable due to an outage">
Inactive item
</ActionList.Item>
Expand Down Expand Up @@ -479,41 +478,43 @@ export const LoadingItem = () => {
}

export const Links = () => (
<ActionList>
<>
<ActionList.Heading as="h1" sx={{fontSize: 1}}>
Details
</ActionList.Heading>
<ActionList.LinkItem href="https://github.com/primer/react#readme">
<ActionList.LeadingVisual>
<BookIcon />
</ActionList.LeadingVisual>
Readme
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/blob/main/LICENSE">
<ActionList.LeadingVisual>
<LawIcon />
</ActionList.LeadingVisual>
MIT License
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/stargazers">
<ActionList.LeadingVisual>
<StarIcon />
</ActionList.LeadingVisual>
<strong>1.5k</strong> stars
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/watchers">
<ActionList.LeadingVisual>
<EyeIcon />
</ActionList.LeadingVisual>
<strong>21</strong> watching
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/network/members">
<ActionList.LeadingVisual>
<RepoForkedIcon />
</ActionList.LeadingVisual>
<strong>225</strong> forks
</ActionList.LinkItem>
</ActionList>
<ActionList>
<ActionList.LinkItem href="https://github.com/primer/react#readme">
<ActionList.LeadingVisual>
<BookIcon />
</ActionList.LeadingVisual>
Readme
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/blob/main/LICENSE">
<ActionList.LeadingVisual>
<LawIcon />
</ActionList.LeadingVisual>
MIT License
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/stargazers">
<ActionList.LeadingVisual>
<StarIcon />
</ActionList.LeadingVisual>
<strong>1.5k</strong> stars
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/watchers">
<ActionList.LeadingVisual>
<EyeIcon />
</ActionList.LeadingVisual>
<strong>21</strong> watching
</ActionList.LinkItem>
<ActionList.LinkItem href="https://github.com/primer/react/network/members">
<ActionList.LeadingVisual>
<RepoForkedIcon />
</ActionList.LeadingVisual>
<strong>225</strong> forks
</ActionList.LinkItem>
</ActionList>
</>
)

export const CustomItemChildren = () => (
Expand Down Expand Up @@ -795,64 +796,85 @@ export const WithCustomTrailingVisuals = () => (
</ActionList>
)

// removing this until CSS Modules FF ships, currently broken in production if button semantic FF is false
// export const WithTrailingAction = () => {
// return (
// <FeatureFlags flags={{primer_react_action_list_item_as_button: true}}>
// <ActionList>
// <ActionList.Item>
// <ActionList.LeadingVisual>
// <FileDirectoryIcon />
// </ActionList.LeadingVisual>
// Item 1 (with default TrailingAction)
// <ActionList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
// </ActionList.Item>
// <ActionList.Item>
// Item 2 (with link TrailingAction)
// <ActionList.TrailingAction as="a" href="#" label="Some action 1" icon={ArrowRightIcon} />
// </ActionList.Item>
// <ActionList.Item>
// Item 3<ActionList.Description>This is an inline description.</ActionList.Description>
// <ActionList.TrailingAction label="Some action 2" icon={BookIcon} />
// </ActionList.Item>
// <ActionList.Item>
// Item 4<ActionList.Description variant="block">This is a block description.</ActionList.Description>
// <ActionList.TrailingAction label="Some action 3" icon={BookIcon} />
// </ActionList.Item>
// <ActionList.Item>
// Item 5<ActionList.Description variant="block">This is a block description.</ActionList.Description>
// <ActionList.TrailingAction label="Some action 4" />
// </ActionList.Item>
// <ActionList.Item>
// Item 6
// <ActionList.TrailingAction href="#" as="a" label="Some action 5" />
// </ActionList.Item>
// <ActionList.LinkItem href="#">
// LinkItem 1
// <ActionList.Description>
// with TrailingAction this is a long description and should not cause horizontal scroll on smaller screen
// sizes
// </ActionList.Description>
// <ActionList.TrailingAction label="Another action" />
// </ActionList.LinkItem>
// <ActionList.LinkItem href="#">
// LinkItem 2
// <ActionList.Description>
// with TrailingVisual this is a long description and should not cause horizontal scroll on smaller screen
// sizes
// </ActionList.Description>
// <ActionList.TrailingVisual>
// <TableIcon />
// </ActionList.TrailingVisual>
// </ActionList.LinkItem>
// <ActionList.Item inactiveText="Unavailable due to an outage">
// Inactive Item<ActionList.Description>With TrailingAction</ActionList.Description>
// <ActionList.TrailingAction as="a" href="#" label="Some action 8" icon={ArrowRightIcon} />
// </ActionList.Item>
// </ActionList>
// </FeatureFlags>
// )
// }
export const ActionListWithButtonSemantics = () => {
return (
<FeatureFlags flags={{primer_react_action_list_item_as_button: true}}>
<ActionList>
<ActionList.Item>Copy link</ActionList.Item>
<ActionList.Item inactiveText="Nothing to quote">Quote reply</ActionList.Item>
<ActionList.Item disabled>Edit comment</ActionList.Item>
<ActionList.Divider />
<ActionList.Item variant="danger">Delete file</ActionList.Item>
<ActionList.LinkItem href="https://github.com/primer/react#readme">
Support
<ActionList.TrailingVisual>
<LinkExternalIcon />
</ActionList.TrailingVisual>
</ActionList.LinkItem>
</ActionList>
</FeatureFlags>
)
}

ActionListWithButtonSemantics.storyName = 'With Button Semantics (Behind feature flag)'

export const WithTrailingAction = () => {
return (
<FeatureFlags flags={{primer_react_action_list_item_as_button: true}}>
<ActionList>
<ActionList.Item>
<ActionList.LeadingVisual>
<FileDirectoryIcon />
</ActionList.LeadingVisual>
Item 1 (with default TrailingAction)
<ActionList.TrailingAction label="Expand sidebar" icon={ArrowLeftIcon} />
</ActionList.Item>
<ActionList.Item>
Item 2 (with link TrailingAction)
<ActionList.TrailingAction as="a" href="#" label="Some action 1" icon={ArrowRightIcon} />
</ActionList.Item>
<ActionList.Item>
Item 3<ActionList.Description>This is an inline description.</ActionList.Description>
<ActionList.TrailingAction label="Some action 2" icon={BookIcon} />
</ActionList.Item>
<ActionList.Item>
Item 4<ActionList.Description variant="block">This is a block description.</ActionList.Description>
<ActionList.TrailingAction label="Some action 3" icon={BookIcon} />
</ActionList.Item>
<ActionList.Item>
Item 5<ActionList.Description variant="block">This is a block description.</ActionList.Description>
<ActionList.TrailingAction label="Some action 4" />
</ActionList.Item>
<ActionList.Item>
Item 6
<ActionList.TrailingAction href="#" as="a" label="Some action 5" />
</ActionList.Item>
<ActionList.LinkItem href="#">
LinkItem 1
<ActionList.Description>
with TrailingAction this is a long description and should not cause horizontal scroll on smaller screen
sizes
</ActionList.Description>
<ActionList.TrailingAction label="Another action" />
</ActionList.LinkItem>
<ActionList.LinkItem href="#">
LinkItem 2
<ActionList.Description>
with TrailingVisual this is a long description and should not cause horizontal scroll on smaller screen
sizes
</ActionList.Description>
<ActionList.TrailingVisual>
<TableIcon />
</ActionList.TrailingVisual>
</ActionList.LinkItem>
<ActionList.Item inactiveText="Unavailable due to an outage">
Inactive Item<ActionList.Description>With TrailingAction</ActionList.Description>
<ActionList.TrailingAction as="a" href="#" label="Some action 8" icon={ArrowRightIcon} />
</ActionList.Item>
</ActionList>
</FeatureFlags>
)
}

export const FullVariant = () => (
<ActionList variant="full">
Expand Down
5 changes: 0 additions & 5 deletions packages/react/src/ActionList/ActionList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ LinkItemPlayground.args = {
active: false,
disabled: false,
id: 'item-1',
variant: 'default',
inactiveText: '',
leadingVisual: null,
loading: false,
Expand All @@ -232,10 +231,6 @@ LinkItemPlayground.argTypes = {
type: 'boolean',
},
},
variant: {
control: 'radio',
options: ['default', 'danger'],
},
role: {
type: 'string',
},
Expand Down
2 changes: 0 additions & 2 deletions packages/react/src/ActionList/Divider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ export const Divider: React.FC<React.PropsWithChildren<ActionListDividerProps>>
marginTop: (theme: Theme) => `calc(${get('space.2')(theme)} - 1px)`,
marginBottom: 2,
listStyle: 'none', // hide the ::marker inserted by browser's stylesheet
marginRight: 'calc(-1 * var(--base-size-8, 8px))',
marginLeft: 'calc(-1 * var(--base-size-8, 8px))',
},
sx as SxProp,
)}
Expand Down
4 changes: 0 additions & 4 deletions packages/react/src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,6 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
bg: selected ? 'fg.muted' : 'var(--control-bgColor-disabled, rgba(175, 184, 193, 0.2))',
borderColor: selected ? 'fg.muted' : 'var(--color-input-disabled-bg, rgba(175, 184, 193, 0.2))',
},
'[data-component="ActionList.Selection"]': {
color: 'primer.fg.disabled',
},
},

// Button reset styles (to support as="button")
Expand Down Expand Up @@ -396,7 +393,6 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
fontWeight: slots.inlineDescription || slots.blockDescription || active ? 'bold' : 'normal',
marginBlockEnd: slots.blockDescription ? '4px' : undefined,
wordBreak: slots.inlineDescription ? 'normal' : 'break-word',
lineHeight: '20px',
}}
>
{childrenWithoutSlots}
Expand Down
3 changes: 0 additions & 3 deletions packages/react/src/ActionList/Visuals.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export const LeadingVisualContainer: React.FC<React.PropsWithChildren<VisualProp
alignItems: 'center',
flexShrink: 0,
marginRight: 2,
color: 'fg.muted',
},
sx as SxProp,
)}
Expand Down Expand Up @@ -70,8 +69,6 @@ export const TrailingVisual: React.FC<React.PropsWithChildren<VisualProps>> = ({
color: getVariantStyles(variant, disabled, inactive).annotationColor,
marginLeft: 2,
fontWeight: 'initial',
display: 'grid',
alignContent: 'center',
'[data-variant="danger"]:hover &, [data-variant="danger"]:active &': {
color: getVariantStyles(variant, disabled, inactive).hoverColor,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,11 @@ export const InactiveItems = () => (
<GearIcon />
</ActionList.LeadingVisual>
</ActionList.LinkItem>
<ActionList.Item onSelect={() => alert('Make a copy clicked')} inactiveText="Unavailable due to an outage">
<ActionList.Item
variant="danger"
onSelect={() => alert('Make a copy clicked')}
inactiveText="Unavailable due to an outage"
>
Make a copy
<ActionList.LeadingVisual>
<CopyIcon />
Expand Down
Loading

0 comments on commit a0ebbde

Please sign in to comment.