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.

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

fix(list): Change default class to activated #3388

Merged
merged 7 commits into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
6 changes: 1 addition & 5 deletions demos/list.html
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
info
</a>
</li>
<li class="mdc-list-item">
<li class="mdc-list-item mdc-list-item--activated">
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be helpful to have separate demos for following cases:

  • singleSelect: with --selected
  • singleSelect: with --activated
  • multiSelect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a demo for --activated.

Copy link
Contributor

Choose a reason for hiding this comment

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

aria-selected and tabindex attr are missing for activated item.

<span class="mdc-list-item__graphic" role="presentation">
<i class="material-icons" aria-hidden="true">folder</i>
</span>
Expand Down Expand Up @@ -1135,10 +1135,6 @@ <h3>Example - Interactive List</h3>
[].slice.call(document.querySelectorAll('.mdc-list')).forEach(function(ele) {
var list = mdc.list.MDCList.attachTo(ele);
list.wrapFocus = true;

if (ele.parentElement.classList.contains('hero')) {
list.singleSelection = true;
}
});

addRippleToListItems(document.getElementById('leading-checkbox-list'));
Expand Down
14 changes: 8 additions & 6 deletions packages/mdc-list/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ OR
### Single Selection List

MDC List can handle selecting/deselecting list elements based on click or keyboard action. When enabled, the `space` and `enter` keys (or `click` event) will trigger an
Copy link
Contributor

Choose a reason for hiding this comment

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

selecting/deselecting

is this still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, when clicked it will deselect the previously selected list item and select the currently selected list item.

single list item to become selected or deselected.
single list item to become selected and any other previous selected element to become deselected.

```html
<ul id="my-list" class="mdc-list" aria-orientation="vertical">
Expand All @@ -153,7 +153,8 @@ list.singleSelection = true;
#### Pre-selected list item

When rendering the list with a pre-selected list item, the list item that needs to be selected should contain
the `mdc-list-item--selected` class and `aria-selected="true"` attribute before creating the list.
the `mdc-list-item--selected` or `mdc-list-item--activated` class and `aria-selected="true"` attribute before
creating the list.

```html
<ul id="my-list" class="mdc-list" aria-orientation="vertical">
Expand Down Expand Up @@ -267,10 +268,10 @@ these should also receive `tabIndex="-1"`.
#### Setup in `singleSelection()`

When implementing a component that will use the single selection variant, the HTML should be modified to include
the `aria-selected` attribute, the `mdc-list-item--selected` class should be added, and the `tabindex` of the selected
element should be `0`. The first list item should have the `tabindex` updated to `-1`. The foundation method
`setSelectedIndex()` should be called with the initially selected element immediately after the foundation is
instantiated.
the `aria-selected` attribute, the `mdc-list-item--selected` or `mdc-list-item--activated` class should be added,
and the `tabindex` of the selected element should be `0`. The first list item should have the `tabindex` updated
Copy link
Contributor

Choose a reason for hiding this comment

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

is it mandator for user to set tabindex -1 explicitly on first element? Doesn't list automatically sets tabindex to -1 to all other list items?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user has marked an element as selected, they should render the first list element to tabindex=-1. The foundation may not be instantiated immediately when the page is loaded (such as in Wiz).

to `-1`. The foundation method `setSelectedIndex()` should be called with the initially selected element immediately
after the foundation is instantiated.

```html
<ul id="my-list" class="mdc-list" aria-orientation="vertical">
Expand Down Expand Up @@ -302,6 +303,7 @@ Method Signature | Description
`setVerticalOrientation(value: Boolean) => void` | Sets the list to an orientation causing the keys used for navigation to change. `true` results in the Up/Down arrow keys being used. `false` results in the Left/Right arrow keys being used.
`setSingleSelection(value: Boolean) => void` | Sets the list to be a selection list. Enables the `enter` and `space` keys for selecting/deselecting a list item.
`setSelectedIndex(index: Number) => void` | Toggles the `selected` state of the list item at index `index`.
`setUseActivated(useActivated: boolean) => void` | Sets the selection logic to apply/remove the `mdc-list-item--activated` class.
`handleFocusIn(evt: Event) => void` | Handles the changing of `tabindex` to `0` for all `button` and `a` elements when a list item receives focus.
`handleFocusOut(evt: Event) => void` | Handles the changing of `tabindex` to `-1` for all `button` and `a` elements when a list item loses focus.
`handleKeydown(evt: Event) => void` | Handles determining if a focus action should occur when a key event is triggered.
Expand Down
1 change: 1 addition & 0 deletions packages/mdc-list/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const cssClasses = {
ROOT: 'mdc-list',
LIST_ITEM_CLASS: 'mdc-list-item',
LIST_ITEM_SELECTED_CLASS: 'mdc-list-item--selected',
LIST_ITEM_ACTIVATED_CLASS: 'mdc-list-item--activated',
};

/** @enum {string} */
Expand Down
27 changes: 15 additions & 12 deletions packages/mdc-list/foundation.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ class MDCListFoundation extends MDCFoundation {
this.isSingleSelectionList_ = false;
/** {number} */
this.selectedIndex_ = -1;
/** {boolean} */
this.useActivatedClass_ = false;
}

/**
Expand All @@ -89,32 +91,33 @@ class MDCListFoundation extends MDCFoundation {
this.isSingleSelectionList_ = value;
}

/**
* Sets the useActivatedClass_ private variable.
* @param {boolean} useActivated
*/
setUseActivatedClass(useActivated) {
this.useActivatedClass_ = useActivated;
}

/** @param {number} index */
setSelectedIndex(index) {
if (index === this.selectedIndex_) {
this.adapter_.removeAttributeForElementIndex(this.selectedIndex_, strings.ARIA_SELECTED);
this.adapter_.removeClassForElementIndex(this.selectedIndex_, cssClasses.LIST_ITEM_SELECTED_CLASS);

// Used to reset the first element to tabindex=0 when deselecting a list item.
// If already on the first list item, leave tabindex at 0.
if (this.selectedIndex_ >= 0) {
this.adapter_.setAttributeForElementIndex(this.selectedIndex_, 'tabindex', -1);
this.adapter_.setAttributeForElementIndex(0, 'tabindex', 0);
}
this.selectedIndex_ = -1;
return;
}

const className = this.useActivatedClass_
? cssClasses.LIST_ITEM_ACTIVATED_CLASS : cssClasses.LIST_ITEM_SELECTED_CLASS;

if (this.selectedIndex_ >= 0) {
this.adapter_.removeAttributeForElementIndex(this.selectedIndex_, strings.ARIA_SELECTED);
this.adapter_.removeClassForElementIndex(this.selectedIndex_, cssClasses.LIST_ITEM_SELECTED_CLASS);
this.adapter_.removeClassForElementIndex(this.selectedIndex_, className);
this.adapter_.setAttributeForElementIndex(this.selectedIndex_, 'tabindex', -1);
}

if (index >= 0 && this.adapter_.getListItemCount() > index) {
this.selectedIndex_ = index;
this.adapter_.setAttributeForElementIndex(this.selectedIndex_, strings.ARIA_SELECTED, true);
this.adapter_.addClassForElementIndex(this.selectedIndex_, cssClasses.LIST_ITEM_SELECTED_CLASS);
this.adapter_.addClassForElementIndex(this.selectedIndex_, className);
this.adapter_.setAttributeForElementIndex(this.selectedIndex_, 'tabindex', 0);

if (this.selectedIndex_ !== 0) {
Expand Down
21 changes: 16 additions & 5 deletions packages/mdc-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class MDCList extends MDCComponent {
this.root_.addEventListener('focusin', this.focusInEventListener_);
this.root_.addEventListener('focusout', this.focusOutEventListener_);
this.layout();
this.initializeListType();
}

layout() {
Expand All @@ -78,6 +79,21 @@ class MDCList extends MDCComponent {
.forEach((ele) => ele.setAttribute('tabindex', -1));
}

initializeListType() {
// Automatically set single selection if selected/activated classes are present.
const preselectedElement =
this.root_.querySelector(`.${cssClasses.LIST_ITEM_ACTIVATED_CLASS}, .${cssClasses.LIST_ITEM_SELECTED_CLASS}`);

if (preselectedElement) {
if (preselectedElement.classList.contains(cssClasses.LIST_ITEM_ACTIVATED_CLASS)) {
this.foundation_.setUseActivatedClass(true);
}

this.singleSelection = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it safe to assume user wants to have single selection mode when list has selected item?

Multi-selectable list can initially have one item selected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is safe to assume that the user has a single selection list when using the selected or activated classes. There isn't a multi-selectable list, but this can be changed if we implement one.

this.selectedIndex = this.listElements_.indexOf(preselectedElement);
}
}

/** @param {boolean} value */
set vertical(value) {
this.foundation_.setVerticalOrientation(value);
Expand All @@ -102,11 +118,6 @@ class MDCList extends MDCComponent {
}

this.foundation_.setSingleSelection(isSingleSelectionList);
const selectedElement = this.root_.querySelector('.mdc-list-item--selected');

if (selectedElement) {
this.selectedIndex = this.listElements_.indexOf(selectedElement);
}
}

/** @param {number} index */
Expand Down
19 changes: 15 additions & 4 deletions test/unit/mdc-list/foundation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ test('#handleKeydown space key is triggered when singleSelection is true selects
td.verify(mockAdapter.setAttributeForElementIndex(0, strings.ARIA_SELECTED, true), {times: 1});
});

test('#handleKeydown space key is triggered 2x when singleSelection is true un-selects the list item', () => {
test('#handleKeydown space key is triggered 2x when singleSelection does not un-select the item.', () => {
const {foundation, mockAdapter} = setupTest();
const preventDefault = td.func('preventDefault');
const target = {classList: ['mdc-list-item']};
Expand All @@ -509,7 +509,8 @@ test('#handleKeydown space key is triggered 2x when singleSelection is true un-s
foundation.handleKeydown(event);

td.verify(preventDefault(), {times: 2});
td.verify(mockAdapter.removeAttributeForElementIndex(0, strings.ARIA_SELECTED), {times: 1});
td.verify(mockAdapter.setAttributeForElementIndex(0, strings.ARIA_SELECTED, true), {times: 1});
td.verify(mockAdapter.removeAttributeForElementIndex(0, strings.ARIA_SELECTED), {times: 0});
});

test('#handleKeydown space key is triggered when singleSelection is true on second ' +
Expand Down Expand Up @@ -544,8 +545,9 @@ test('#handleKeydown space key is triggered 2x when singleSelection is true on s
foundation.handleKeydown(event);

td.verify(preventDefault(), {times: 2});
td.verify(mockAdapter.setAttributeForElementIndex(0, 'tabindex', 0), {times: 1});
td.verify(mockAdapter.setAttributeForElementIndex(1, 'tabindex', -1), {times: 1});
td.verify(mockAdapter.setAttributeForElementIndex(1, strings.ARIA_SELECTED, true), {times: 1});
td.verify(mockAdapter.setAttributeForElementIndex(1, 'tabindex', 0), {times: 1});
td.verify(mockAdapter.setAttributeForElementIndex(0, 'tabindex', -1), {times: 1});
});

test('#handleKeydown space key is triggered and focused is moved to a different element', () => {
Expand Down Expand Up @@ -646,3 +648,12 @@ test('#focusLastElement is called when the list is empty does not focus an eleme

td.verify(mockAdapter.focusItemAtIndex(td.matchers.anything()), {times: 0});
});

test('#setUseActivatedClass causes setSelectedIndex to use the --activated class', () => {
const {foundation, mockAdapter} = setupTest();
td.when(mockAdapter.getListItemCount()).thenReturn(3);
foundation.setUseActivatedClass(true);
foundation.setSelectedIndex(1);

td.verify(mockAdapter.addClassForElementIndex(1, cssClasses.LIST_ITEM_ACTIVATED_CLASS), {times: 1});
});
30 changes: 24 additions & 6 deletions test/unit/mdc-list/mdc-list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,30 @@ test('component calls setVerticalOrientation(true) on the foundation if aria-ori
td.verify(mockFoundation.setVerticalOrientation(true), {times: 1});
});

test('#initializeListType sets the selectedIndex if a list item has the --selected class', () => {
const {root, component, mockFoundation} = setupTest();
root.querySelector('.mdc-list-item').classList.add(MDCListFoundation.cssClasses.LIST_ITEM_SELECTED_CLASS);
component.initializeListType();
td.verify(mockFoundation.setSelectedIndex(0), {times: 1});
td.verify(mockFoundation.setSingleSelection(true), {times: 1});
});

test('#initializeListType sets the selectedIndex if a list item has the --activated class', () => {
const {root, component, mockFoundation} = setupTest();
root.querySelector('.mdc-list-item').classList.add(MDCListFoundation.cssClasses.LIST_ITEM_ACTIVATED_CLASS);
component.initializeListType();
td.verify(mockFoundation.setSelectedIndex(0), {times: 1});
td.verify(mockFoundation.setSingleSelection(true), {times: 1});
});

test('#initializeListType calls the foundation if the --activated class is present', () => {
const {root, component, mockFoundation} = setupTest();
root.querySelector('.mdc-list-item').classList.add(MDCListFoundation.cssClasses.LIST_ITEM_ACTIVATED_CLASS);
component.initializeListType();
td.verify(mockFoundation.setUseActivatedClass(true), {times: 1});
td.verify(mockFoundation.setSingleSelection(true), {times: 1});
});

test('#adapter.getListItemCount returns correct number of list items', () => {
const {root, component} = setupTest();
document.body.appendChild(root);
Expand Down Expand Up @@ -259,12 +283,6 @@ test('wrapFocus calls setWrapFocus on foundation', () => {
td.verify(mockFoundation.setWrapFocus(true), {times: 1});
});

test('singleSelection true sets the selectedIndex if a list item has the --selected class', () => {
const {root, component, mockFoundation} = setupTest();
root.querySelector('.mdc-list-item').classList.add(MDCListFoundation.cssClasses.LIST_ITEM_SELECTED_CLASS);
component.singleSelection = true;
td.verify(mockFoundation.setSelectedIndex(0), {times: 1});
});

test('singleSelection true sets the click handler from the root element', () => {
const {root, component, mockFoundation} = setupTest();
Expand Down