Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(list): Use states mixins; change padding behavior to support them #1737

Merged
merged 13 commits into from
Dec 14, 2017
Merged

feat(list): Use states mixins; change padding behavior to support them #1737

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c9d5e21
feat(list): Use states mixins; change padding behavior to support them
Dec 8, 2017
2433b60
WIP Add comments
Dec 12, 2017
394f665
WIP Undo demo page markup change
Dec 12, 2017
f76d297
WIP Prepend demo- to demo/index classes
Dec 12, 2017
2159646
WIP Add backticks around tabindex in readme
Dec 12, 2017
4a02dd4
Revert "WIP Prepend demo- to demo/index classes"
Dec 13, 2017
9bdd499
Revert "WIP Undo demo page markup change"
Dec 13, 2017
d01f559
WIP Re-apply new DOM with roles
Dec 13, 2017
2bc3f81
WIP Replace hard-coded color value with palette variable
Dec 13, 2017
41a54d3
Merge branch 'master' into feat/states-list
Dec 13, 2017
3165a9d
WIP Remove apparently-unused class
Dec 13, 2017
091e0b7
Merge branch 'master' into feat/states-list
Dec 13, 2017
d98d280
Merge branch 'master' into feat/states-list
kfranqueiro Dec 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
220 changes: 110 additions & 110 deletions demos/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,216 +37,216 @@
</header>
<main>
<nav class="mdc-toolbar-fixed-adjust">
<ul class="catalog-list mdc-list mdc-list--two-line">
<li class="mdc-list-item">
<div class="catalog-list mdc-list mdc-list--two-line">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any accessibility implications to changing the DOM structure from
<nav><ul><li> to <nav><div><a>?

E.g., will dictation software describe or categorize them differently?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wonder if this will effect page rank at all. Is the crawler smart enough to parse all this without the help of a site map or something. Can someone more enlightened chime in?

Copy link
Contributor Author

@kfranqueiro kfranqueiro Dec 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern as to why I did this was to make the entire rows clickable because the hover state will make users think the whole line is clickable, when in fact right now it's not. If we're concerned about the markup, what I can do instead is revert this and simply set the parent to non-interactive so that this UX issue doesn't occur.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://html.spec.whatwg.org/multipage/sections.html#the-nav-element

Seems like <nav> is enough for user agents, but no mention of anything else in the living standard.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, maybe we should just add role="list" and role="listitem" and call it a day.

For the record, I totally understand why you made this change, and I love the UX improvement it provides. I just want to make sure we don't accidentally break accessibility or SEO 🙂

The change in DOM structure might affect PageRank, but I'm not certain. I've generally seen <nav> elements contain a <ul><li>, which suggests it's a standard practice for some reason, but I don't know if it's for SEO or accessibility (or both).

From doing a few quick searches, I don't think our SEO can get any worse - Google Search doesn't appear to display any sitelinks for our demo site (likely because we have so many different navigation destinations), and the material.io catalog site seems to get ranked higher. So I'm not too worried about SEO impact.

Once I get a Windows machine, I can definitively test the accessibility aspect in JAWS, but for now, I suspect adding the role attributes ought to suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I saw this conversation unfolding I had reverted the new DOM structure, but I've put it back and added roles.

<a href="button.html" class="mdc-list-item">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ 🎉

<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_button_24px.svg" /></span>
<a href="button.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Button
<span class="mdc-list-item__secondary-text">Raised and flat buttons</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="card.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_card_24px.svg" /></span>
<a href="card.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Card
<span class="mdc-list-item__secondary-text">Various card layout styles</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="checkbox.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_selection_control_24px.svg" /></span>
<a href="checkbox.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Checkbox
<span class="mdc-list-item__secondary-text">Multi-selection controls</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="dialog.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_dialog_24px.svg" /></span>
<a href="dialog.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Dialog
<span class="mdc-list-item__secondary-text">Secondary text</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="drawer/temporary-drawer.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_side_navigation_24px.svg" /></span>
<a href="drawer/temporary-drawer.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Drawer
<span class="mdc-list-item__secondary-text">Temporary</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="drawer/persistent-drawer.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_side_navigation_24px.svg" /></span>
<a href="drawer/persistent-drawer.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Drawer
<span class="mdc-list-item__secondary-text">Persistent</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="drawer/permanent-drawer-above-toolbar.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_side_navigation_24px.svg" /></span>
<a href="drawer/permanent-drawer-above-toolbar.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Drawer
<span class="mdc-list-item__secondary-text">Permanent drawer above toolbar</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="drawer/permanent-drawer-below-toolbar.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_side_navigation_24px.svg" /></span>
<a href="drawer/permanent-drawer-below-toolbar.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Drawer
<span class="mdc-list-item__secondary-text">Permanent drawer below toolbar</span>
</a>
</li>
<li class="mdc-list-item">
</span>
</a>
<a href="elevation.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_shadow_24px.svg" /></span>
<a href="elevation.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Elevation
<span class="mdc-list-item__secondary-text">Shadow for different elevations</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="fab.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_button_24px.svg" /></span>
<a href="fab.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Floating action button
<span class="mdc-list-item__secondary-text">The primary action in an application</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="grid-list.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_card_24px.svg" /></span>
<a href="grid-list.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Grid list
<span class="mdc-list-item__secondary-text">2D grid layouts</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="icon-toggle.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_component_24px.svg" /></span>
<a href="icon-toggle.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Icon toggle
<span class="mdc-list-item__secondary-text">Toggling icon states</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="layout-grid.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_card_24px.svg" /></span>
<a href="layout-grid.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Layout grid
<span class="mdc-list-item__secondary-text">Grid and gutter support</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="linear-progress.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_progress_activity.svg" /></span>
<a href="linear-progress.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Linear progress
<span class="mdc-list-item__secondary-text">Fills from 0% to 100%, represented by bars</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="list.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_list_24px.svg" /></span>
<a href="list.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
List
<span class="mdc-list-item__secondary-text">Item layouts in lists</span>
</a>
</li>
</span>
</a>


<li class="mdc-list-item">
<a href="radio.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_radio_button_24px.svg" /></span>
<a href="radio.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Radio buttons
<span class="mdc-list-item__secondary-text">Single selection controls</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="ripple.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_ripple_24px.svg" /></span>
<a href="ripple.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Ripple
<span class="mdc-list-item__secondary-text">Touch ripple</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="select.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_menu_24px.svg" /></span>
<a href="select.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Select
<span class="mdc-list-item__secondary-text">Popover selection menus</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="simple-menu.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_menu_24px.svg" /></span>
<a href="simple-menu.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Simple Menu
<span class="mdc-list-item__secondary-text">Pop over menus</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="slider.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/slider.svg" /></span>
<a href="slider.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Slider
<span class="mdc-list-item__secondary-text">Range Controls</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="snackbar.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_toast_24px.svg" /></span>
<a href="snackbar.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Snackbar
<span class="mdc-list-item__secondary-text">Transient messages</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="switch.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_switch_24px.svg" /></span>
<a href="switch.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Switch
<span class="mdc-list-item__secondary-text">On off switches</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="tabs.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_tabs_24px.svg" /></span>
<a href="tabs.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Tabs
<span class="mdc-list-item__secondary-text">Tabs with support for icon and text labels</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="text-field.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_text_field_24px.svg" /></span>
<a href="text-field.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Text field
<span class="mdc-list-item__secondary-text">Single and multiline text fields</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="theme.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_theme_24px.svg" /></span>
<a href="theme.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Theme
<span class="mdc-list-item__secondary-text">Using primary and secondary colors</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="toolbar/index.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_toolbar_24px.svg" /></span>
<a href="toolbar/index.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Toolbar
<span class="mdc-list-item__secondary-text">Header and footers</span>
</a>
</li>
</span>
</a>

<li class="mdc-list-item">
<a href="typography.html" class="mdc-list-item">
<span class="catalog-list-icon mdc-list-item__start-detail"><img class="catalog-component-icon" src="/images/ic_typography_24px.svg" /></span>
<a href="typography.html" class="mdc-list-item__text">
<span class="mdc-list-item__text">
Typography
<span class="mdc-list-item__secondary-text">Type hierarchy</span>
</a>
</li>
</ul>
</span>
</a>
</div>
</nav>
</main>
<script src="/assets/material-components-web.js"></script>
Expand Down
14 changes: 4 additions & 10 deletions demos/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,10 @@
@import "./common";
@import "../packages/mdc-list/mdc-list";

.catalog-list {
padding-left: 28px;
}
.catalog-list-icon {
margin-right: 24px;
.catalog-list a {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we possibly rename this to .demo-catalog-list?

I prefer that convention because it makes it obvious that the CSS class is only used
by a demo page, and not for any other purpose (as opposed to, say, .material-icons).

color: #212121;
}

ul.catalog-list {
a {
text-decoration: none;
color: #212121;
}
.catalog-list-icon {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

margin: 0 24px 0 12px;
}
6 changes: 3 additions & 3 deletions demos/list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ <h3 class="mdc-list-group__subheader">Files</h3>
<section>
<h2>Single-Line List</h2>
<section>
<h3>Text only</h3>
<ul class="mdc-list demo-list">
<h3>Text only, non-interactive (no states)</h3>
<ul class="mdc-list mdc-list--non-interactive demo-list">
<li class="mdc-list-item">Single-line item</li>
<li class="mdc-list-item">Single-line item</li>
<li class="mdc-list-item">Single-line item</li>
Expand Down Expand Up @@ -1025,7 +1025,7 @@ <h3 class="mdc-list-group__subheader">Folders</h3>
</a>
</li>
</ul>
<hr class="mdc-list-divider mdc-list-divider--inset">
<hr class="mdc-list-divider mdc-list-divider--inset mdc-list-divider--padded">
<h3 class="mdc-list-group__subheader">Files</h3>
<ul class="mdc-list mdc-list--two-line mdc-list--avatar-list demo-list demo-list--with-avatars demo-list--icon-placeholders">
<li class="mdc-list-item">
Expand Down
Loading