Skip to content

Commit

Permalink
[WNMGDS-2486] Fix high specificity of list normalization rules and fi…
Browse files Browse the repository at this point in the history
…x bare list styles (#2649)

* Fix high specificity of our list normalization rules and fix bare class

1. Use `:where` in our `ol:not([role])` selector so our reset styles don't have a greater specificity than application-applied classes!
2. We actually want to let teams apply a `role="list"` to `ds-c-list--bare` lists, which are a little bit different historically from completely unstyled lists. Historically, we've applied the standard item spacing to bare lists vs completely unstyled lists. So I've modified our reset rules to make sure bare lists get the base normalization styles applied.

(I also updated our list story to show what applying `role="list"` to a bare list would look like

* Add a comment about the change

* Update VRT refs

* Get rid of redundant stories and update the doc page examples

* Change our strategy so we don't reference classes in the reset

Having exceptions for the bare list class makes it overly complex and less clear
  • Loading branch information
pwolfert authored Aug 24, 2023
1 parent 3f09f12 commit 43f627b
Show file tree
Hide file tree
Showing 106 changed files with 77 additions and 55 deletions.
54 changes: 24 additions & 30 deletions packages/design-system/src/components/List/List.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,34 @@ export default meta;

type Story = StoryObj;

const listMarkup = (type: string) => {
const label = type.charAt(0).toUpperCase() + type.slice(1);
let className = 'ds-c-list';
let Tag = 'ul' as any;

if (type === 'unstyled') className = 'ds-c-list ds-c-list--bare';
if (type === 'ordered') Tag = 'ol' as any;

return (
<>
<Tag className={className} aria-labelledby={`${type}-list-id`}>
<li>{label} list item 1</li>
<li>{label} list item 2</li>
</Tag>
</>
);
};
const ListItems = ({ label }) => (
<>
<li>{label} list item 1</li>
<li>{label} list item 2</li>
</>
);
const UnorderedList = () => (
<ul className="ds-c-list">
<ListItems label="Unordered" />
</ul>
);
const OrderedList = () => (
<ol className="ds-c-list">
<ListItems label="Ordered" />
</ol>
);
const BareList = () => (
<ul className="ds-c-list ds-c-list--bare" role="list">
<ListItems label="Bare" />
</ul>
);

export const AllLists: Story = {
render: () => (
<>
{listMarkup('unordered')}
{listMarkup('ordered')}
{listMarkup('unstyled')}
<UnorderedList />
<OrderedList />
<BareList />
</>
),
};
Expand All @@ -50,13 +54,3 @@ export const AllListsOnDark: Story = {
layout: 'fullscreen',
},
};

export const unorderedList: Story = {
render: () => <>{listMarkup('unordered')}</>,
};
export const orderedList: Story = {
render: () => <>{listMarkup('ordered')}</>,
};
export const unstyledList: Story = {
render: () => <>{listMarkup('unstyled')}</>,
};
10 changes: 9 additions & 1 deletion packages/design-system/src/styles/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,15 @@
* Lists
*/

@mixin ds-unstyled-list {
// Base properties for styling lists with flexbox, including a standard gap
// between items
@mixin flex-list {
display: flex;
flex-direction: column;
gap: 0.5em;
}

@mixin unstyled-list {
list-style: none;
margin: 0;
padding: 0;
Expand Down
14 changes: 6 additions & 8 deletions packages/design-system/src/styles/_reset.scss
Original file line number Diff line number Diff line change
Expand Up @@ -130,27 +130,25 @@ hr {
// that create the exception to the default styling because there are other
// legitimate roles that could be applied like `role="listbox"`.

ol:not([role]),
ul:not([role]) {
ol:not(:where([role])),
ul:not(:where([role])) {
// Relying on `flex` to set `gap` spacing size on li elements
// `gap` ensures even spacing and doesn't interfere with layout
// if list given a `row` direction
display: flex;
flex-direction: column;
gap: 0.5em;
@include mixins.flex-list;
margin-block: 1em 0;
padding-inline-start: 2em;
}

// Tie the application of unstyled-list styles to inclusion of a role attribute
// that will "fix" Safari behavior outlined in the guidance. See above comment.
[role='list']:where(ul, ol) {
@include mixins.ds-unstyled-list;
@include mixins.unstyled-list;
}

li {
> ul:not([role]),
> ol:not([role]) {
> ul:not(:where([role])),
> ol:not(:where([role])) {
margin-block: 0.5em 0;
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/design-system/src/styles/components/_Dropdown.scss
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
}

.ds-c-dropdown__menu {
@include mixins.ds-unstyled-list;
@include mixins.unstyled-list;
// Allows for keyboard navigation of dropdown options, i.e., scrolling through options via arrow keys
max-height: calc(var(--max-height) - var(--text-input__border-width));
overflow-y: auto;
Expand All @@ -89,7 +89,7 @@
}

.ds-c-dropdown__menu-item-group ul[role='presentation'] {
@include mixins.ds-unstyled-list;
@include mixins.unstyled-list;
}

.ds-c-dropdown__menu-item-group .ds-c-dropdown__menu-item {
Expand Down
8 changes: 7 additions & 1 deletion packages/design-system/src/styles/components/_List.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,11 @@
}

.ds-c-list--bare {
@include mixins.ds-unstyled-list;
// Bare lists should have `role="list"` applied to them, which makes them
// exempt from the regular list-normalization styles. Duplicate them here,
// because "Bare" lists should have the same spacing as our other lists but
// not have bullets or numbers; they're something different than completely
// unstyled lists.
@include mixins.flex-list;
@include mixins.unstyled-list;
}
42 changes: 29 additions & 13 deletions packages/docs/content/foundation/typography/list.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,47 @@ import loremIpsumGenerator from '../../../src/helpers/loremIpsumGenerator';

<EmbeddedExample>

<ul class="ds-c-list" aria-labelledby="unordered-list-id">
<li>List item 1</li>
<li>List item 2</li>
</ul>
<ul class="ds-c-list">
<li>Unordered list item 1</li>
<li>Unordered list item 2</li>
</ul>

</EmbeddedExample>

### Ordered list
`.ds-c-list`

<EmbeddedExample>
<ol class="ds-c-list" aria-labelledby="ordered-list-id">
<li>List item 1</li>
<li>List item 2</li>
</ol>

<ol class="ds-c-list">
<li>Ordered list item 1</li>
<li>Ordered list item 2</li>
</ol>

</EmbeddedExample>

### Unstyled list
### Bare list
`.ds-c-list .ds-c-list--bare`

<EmbeddedExample>
<ul role="list" class="ds-c-list ds-c-list--bare" aria-labelledby="unstyled-list-id">
<li>List item 1</li>
<li>List item 2</li>
</ul>

<ul role="list" class="ds-c-list ds-c-list--bare">
<li>Bare list item 1</li>
<li>Bare list item 2</li>
</ul>

</EmbeddedExample>

### Unstyled list
`role="list"`

<EmbeddedExample>

<ul role="list">
<li>Unstyled list item 1</li>
<li>Unstyled list item 2</li>
</ul>

</EmbeddedExample>

## Guidance
Expand Down
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.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.

0 comments on commit 43f627b

Please sign in to comment.