From 3d22a457598cb36a9a4d6f169ef74be989fe8e6e Mon Sep 17 00:00:00 2001 From: Aditi Mandal Date: Thu, 18 Aug 2016 10:42:22 -0700 Subject: [PATCH 01/11] Updating the group-by icon --- src/common/_common.scss | 1 + src/common/_fabricExtraIcons.scss | 2 ++ src/components/DetailsList/DetailsHeader.tsx | 2 +- .../DetailsListPage/examples/DetailsList.Advanced.Example.tsx | 2 +- 4 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 src/common/_fabricExtraIcons.scss diff --git a/src/common/_common.scss b/src/common/_common.scss index 5817539e378b9..7411c91206020 100644 --- a/src/common/_common.scss +++ b/src/common/_common.scss @@ -4,4 +4,5 @@ @import './themeOverrides'; @import './focusBorder'; @import './semanticColorVariables'; +@import './fabricExtraIcons'; diff --git a/src/common/_fabricExtraIcons.scss b/src/common/_fabricExtraIcons.scss new file mode 100644 index 0000000000000..79b050863cca6 --- /dev/null +++ b/src/common/_fabricExtraIcons.scss @@ -0,0 +1,2 @@ +.ms-Icon--groupingAscending:before { content: '\e31d'; } +.ms-Icon--groupingDescending:before { content: '\e31e'; } \ No newline at end of file diff --git a/src/components/DetailsList/DetailsHeader.tsx b/src/components/DetailsList/DetailsHeader.tsx index 890b95b4e5cee..166a8c817c937 100644 --- a/src/components/DetailsList/DetailsHeader.tsx +++ b/src/components/DetailsList/DetailsHeader.tsx @@ -164,7 +164,7 @@ export class DetailsHeader extends BaseComponent + ) } { column.iconClassName && ( diff --git a/src/demo/pages/DetailsListPage/examples/DetailsList.Advanced.Example.tsx b/src/demo/pages/DetailsListPage/examples/DetailsList.Advanced.Example.tsx index 9a9ad6d12e762..2d71ccd519bdc 100644 --- a/src/demo/pages/DetailsListPage/examples/DetailsList.Advanced.Example.tsx +++ b/src/demo/pages/DetailsListPage/examples/DetailsList.Advanced.Example.tsx @@ -373,7 +373,7 @@ export class DetailsListAdvancedExample extends React.Component this._onGroupByColumn(column) From 3545de25ffae10ed6862f8eb97de7c7910bf7f07 Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Fri, 19 Aug 2016 11:34:30 -0700 Subject: [PATCH 02/11] Added initial focus to callout --- src/components/Callout/Callout.Props.ts | 8 ++++++++ src/components/Callout/Callout.tsx | 13 +++++++++++-- src/components/Popup/Popup.tsx | 5 +++-- .../CalloutPage/examples/Callout.Basic.Example.tsx | 1 + .../examples/Callout.Nested.Example.tsx | 3 ++- src/utilities/focus.ts | 14 ++++++++++++++ 6 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/components/Callout/Callout.Props.ts b/src/components/Callout/Callout.Props.ts index 9135fea56d796..9fef8081cb59f 100644 --- a/src/components/Callout/Callout.Props.ts +++ b/src/components/Callout/Callout.Props.ts @@ -59,6 +59,14 @@ export interface ICalloutProps extends React.Props, IPositionProps { * If true do not render on a new layer. If false render on a new layer. */ doNotLayer?: boolean; + + /** + * If true then the callout will attempt to focus the first focusable element that it contains. + * If it does not find an element then focus will not be given to the callout. + * This means that it's the contents responsibility to either set focus or have + * focusable items. + */ + setInitialFocus?: boolean; } export interface ILink { diff --git a/src/components/Callout/Callout.tsx b/src/components/Callout/Callout.tsx index 4c2c0b7c8c6fb..f45591a49d1e1 100644 --- a/src/components/Callout/Callout.tsx +++ b/src/components/Callout/Callout.tsx @@ -5,6 +5,8 @@ import { Layer } from '../../Layer'; import { css } from '../../utilities/css'; import { EventGroup } from '../../utilities/eventGroup/EventGroup'; import { getRelativePositions, IPositionInfo } from '../../utilities/positioning'; +import { focusFirstFocusable } from '../../utilities/focus'; +import { Popup } from '../Popup/index'; import './Callout.scss'; const BEAK_ORIGIN_POSITION = { top: 0, left: 0 }; @@ -69,9 +71,12 @@ export class Callout extends React.Component { ref={ (callout: HTMLDivElement) => this._calloutElement = callout } > { isBeakVisible && targetElement ? (
) : (null) } -
+ this.dismiss() } + shouldRestoreFocus={ true }> { children } -
+
); @@ -110,6 +115,10 @@ export class Callout extends React.Component { this._events.on(window, 'focus', this._dismissOnLostFocus, true); this._events.on(window, 'click', this._dismissOnLostFocus, true); + if (this.props.setInitialFocus) { + focusFirstFocusable(this._calloutElement, this._calloutElement, true); + } + if (this.props.onLayerMounted) { this.props.onLayerMounted(); } diff --git a/src/components/Popup/Popup.tsx b/src/components/Popup/Popup.tsx index 1c76f88641c01..3ad48e8c73ed3 100644 --- a/src/components/Popup/Popup.tsx +++ b/src/components/Popup/Popup.tsx @@ -45,12 +45,13 @@ export class Popup extends BaseComponent { return (
+ aria-desribedby={ ariaDescribedBy }> + { this.props.children } +
); } diff --git a/src/demo/pages/CalloutPage/examples/Callout.Basic.Example.tsx b/src/demo/pages/CalloutPage/examples/Callout.Basic.Example.tsx index 3b2199c1e0b0e..c0384ed99fcd7 100644 --- a/src/demo/pages/CalloutPage/examples/Callout.Basic.Example.tsx +++ b/src/demo/pages/CalloutPage/examples/Callout.Basic.Example.tsx @@ -38,6 +38,7 @@ export class CalloutBasicExample extends React.Component

diff --git a/src/demo/pages/CalloutPage/examples/Callout.Nested.Example.tsx b/src/demo/pages/CalloutPage/examples/Callout.Nested.Example.tsx index 1dd84373e5ca3..ad10fde4e5dd5 100644 --- a/src/demo/pages/CalloutPage/examples/Callout.Nested.Example.tsx +++ b/src/demo/pages/CalloutPage/examples/Callout.Nested.Example.tsx @@ -38,7 +38,8 @@ export class CalloutNestedExample extends React.Component { this._onDismiss(ev); } } + onDismiss={ (ev: any) => { this._onDismiss(ev); } } + setInitialFocus={ true } >

diff --git a/src/utilities/focus.ts b/src/utilities/focus.ts index 7cde3cad0de6c..dddd11de9786a 100644 --- a/src/utilities/focus.ts +++ b/src/utilities/focus.ts @@ -20,6 +20,20 @@ export function getLastFocusable( return getPreviousElement(rootElement, currentElement, true, false, true, includeElementsInFocusZones); } +// Attempts to focus the first focusable element. If it successfully focuses it will return true. Otherwise it will return false. +export function focusFirstFocusable( + rootElement: HTMLElement, + currentElement: HTMLElement, + includeElementsInFocusZones?: boolean): boolean { + let element: HTMLElement = getNextElement(rootElement, currentElement, true, false, false, includeElementsInFocusZones); + + if (element) { + element.focus(); + return true; + } + return false; +} + /** Traverse to find the previous element. */ export function getPreviousElement( rootElement: HTMLElement, From e27580a611e6e3fe7270f9575929310d44a27eee Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Fri, 19 Aug 2016 14:28:27 -0700 Subject: [PATCH 03/11] changed focus method to be more clear. Added more jsdoc comments --- src/components/Callout/Callout.Props.ts | 3 ++- src/components/Callout/Callout.tsx | 4 ++-- src/utilities/focus.ts | 14 ++++++++------ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/Callout/Callout.Props.ts b/src/components/Callout/Callout.Props.ts index 9fef8081cb59f..3bd5f089c5a0e 100644 --- a/src/components/Callout/Callout.Props.ts +++ b/src/components/Callout/Callout.Props.ts @@ -62,9 +62,10 @@ export interface ICalloutProps extends React.Props, IPositionProps { /** * If true then the callout will attempt to focus the first focusable element that it contains. - * If it does not find an element then focus will not be given to the callout. + * If it doesn't find an element, no focus will be set and the method will return false. * This means that it's the contents responsibility to either set focus or have * focusable items. + * @returns True if focus was set, false if it was not. */ setInitialFocus?: boolean; } diff --git a/src/components/Callout/Callout.tsx b/src/components/Callout/Callout.tsx index f45591a49d1e1..aaa6a7793df6a 100644 --- a/src/components/Callout/Callout.tsx +++ b/src/components/Callout/Callout.tsx @@ -5,7 +5,7 @@ import { Layer } from '../../Layer'; import { css } from '../../utilities/css'; import { EventGroup } from '../../utilities/eventGroup/EventGroup'; import { getRelativePositions, IPositionInfo } from '../../utilities/positioning'; -import { focusFirstFocusable } from '../../utilities/focus'; +import { focusFirstChild } from '../../utilities/focus'; import { Popup } from '../Popup/index'; import './Callout.scss'; @@ -116,7 +116,7 @@ export class Callout extends React.Component { this._events.on(window, 'click', this._dismissOnLostFocus, true); if (this.props.setInitialFocus) { - focusFirstFocusable(this._calloutElement, this._calloutElement, true); + focusFirstChild(this._calloutElement); } if (this.props.onLayerMounted) { diff --git a/src/utilities/focus.ts b/src/utilities/focus.ts index dddd11de9786a..a77f3181031ba 100644 --- a/src/utilities/focus.ts +++ b/src/utilities/focus.ts @@ -20,12 +20,14 @@ export function getLastFocusable( return getPreviousElement(rootElement, currentElement, true, false, true, includeElementsInFocusZones); } -// Attempts to focus the first focusable element. If it successfully focuses it will return true. Otherwise it will return false. -export function focusFirstFocusable( - rootElement: HTMLElement, - currentElement: HTMLElement, - includeElementsInFocusZones?: boolean): boolean { - let element: HTMLElement = getNextElement(rootElement, currentElement, true, false, false, includeElementsInFocusZones); +/** + * Attempts to focus the first focusable element that is a child or child's child of the rootElement. + * @return True if focus was set, false if it was not. + * @param {HTMLElement} rootElement - element to start the search for a focusable child. + */ +export function focusFirstChild( + rootElement: HTMLElement): boolean { + let element: HTMLElement = getNextElement(rootElement, rootElement, true, false, false, true); if (element) { element.focus(); From f448447d842d2d8fe9b939ce4305b685721db543 Mon Sep 17 00:00:00 2001 From: David Zearing Date: Fri, 19 Aug 2016 17:39:05 -0700 Subject: [PATCH 04/11] Update .pullapprove.yml (#166) * Update .pullapprove.yml * Update .pullapprove.yml * Update .pullapprove.yml --- .pullapprove.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.pullapprove.yml b/.pullapprove.yml index df88e9e2688ab..b113477b13f3d 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -1,6 +1,6 @@ approve_by_comment: true approve_regex: ^Approved -reject_regex: ^Rejected +reject_regex: ^Blocked reset_on_push: false reject_value: -1 reviewers: @@ -15,5 +15,6 @@ reviewers: - aditima - yiminwu - antonlabunets + - cschlechty name: pullapprove required: 1 From a164b0f1cf16fb09c257924bd0899128ad5313cd Mon Sep 17 00:00:00 2001 From: David Zearing Date: Sun, 21 Aug 2016 16:34:43 -0700 Subject: [PATCH 05/11] Adding focus to command bar contract. (#165) --- src/components/CommandBar/CommandBar.Props.ts | 7 +++++++ src/components/CommandBar/CommandBar.tsx | 11 ++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/components/CommandBar/CommandBar.Props.ts b/src/components/CommandBar/CommandBar.Props.ts index 5cf81c28757e7..33b04d32f82bf 100644 --- a/src/components/CommandBar/CommandBar.Props.ts +++ b/src/components/CommandBar/CommandBar.Props.ts @@ -1,6 +1,13 @@ import * as React from 'react'; import { IContextualMenuItem } from '../ContextualMenu/index'; +export interface ICommandBar { + /** + * Sets focus to the active command in the list. + */ + focus(): void; +} + export interface ICommandBarProps extends React.HTMLProps { /** * Whether or not the search box is visible diff --git a/src/components/CommandBar/CommandBar.tsx b/src/components/CommandBar/CommandBar.tsx index f22985e1460a2..2cb556f9ce265 100644 --- a/src/components/CommandBar/CommandBar.tsx +++ b/src/components/CommandBar/CommandBar.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { ICommandBarProps } from './CommandBar.Props'; +import { ICommandBar, ICommandBarProps } from './CommandBar.Props'; import { FocusZone, FocusZoneDirection } from '../../FocusZone'; import { ContextualMenu, IContextualMenuItem } from '../../ContextualMenu'; import { EventGroup } from '../../utilities/eventGroup/EventGroup'; @@ -21,7 +21,7 @@ export interface ICommandBarState { renderedFarItems?: IContextualMenuItem[]; } -export class CommandBar extends React.Component { +export class CommandBar extends React.Component implements ICommandBar { public static defaultProps = { items: [], overflowItems: [], @@ -34,6 +34,7 @@ export class CommandBar extends React.Component { searchBox } - +

{ renderedItems.map((item, index) => ( this._renderItemInCommandBar(item, index, expandedMenuItemKey) @@ -139,6 +140,10 @@ export class CommandBar extends React.Component Date: Sun, 21 Aug 2016 19:28:17 -0700 Subject: [PATCH 06/11] SelectionZone: adding a ton of unit tests and fine tuning all the edge cases (#172) * Allowing specific elements to immediately invoke the item. * Addding tests. * Adding significantly more unit test coverage. * Updates to docs. * Removing unnecessary change. * Updating detailslist basic example with correct selection example. --- src/components/Link/Link.scss | 4 + .../examples/DetailsList.Basic.Example.tsx | 23 +- .../pages/SelectionPage/SelectionPage.tsx | 25 +- .../examples/Selection.Basic.Example.tsx | 9 +- .../examples/Selection.Example.scss | 4 + src/utilities/selection/Selection.ts | 15 +- .../selection/SelectionZone.test.tsx | 224 ++++++++++ src/utilities/selection/SelectionZone.tsx | 396 +++++++++++------- src/utilities/selection/interfaces.ts | 4 +- 9 files changed, 540 insertions(+), 164 deletions(-) create mode 100644 src/utilities/selection/SelectionZone.test.tsx diff --git a/src/components/Link/Link.scss b/src/components/Link/Link.scss index def5dffe1fef2..5658634d3f513 100644 --- a/src/components/Link/Link.scss +++ b/src/components/Link/Link.scss @@ -52,5 +52,9 @@ BUTTON.ms-Link { display: inline; padding: 0; margin: 0; + + width: inherit; + overflow: inherit; + text-overflow: inherit; } diff --git a/src/demo/pages/DetailsListPage/examples/DetailsList.Basic.Example.tsx b/src/demo/pages/DetailsListPage/examples/DetailsList.Basic.Example.tsx index 2d5fb94673b14..14130c95d8f85 100644 --- a/src/demo/pages/DetailsListPage/examples/DetailsList.Basic.Example.tsx +++ b/src/demo/pages/DetailsListPage/examples/DetailsList.Basic.Example.tsx @@ -5,7 +5,8 @@ import { DetailsList, MarqueeSelection, Selection, - TextField + TextField, + Link } from '../../../../index'; import { createListItems } from '../../../utilities/data'; @@ -19,6 +20,7 @@ export class DetailsListBasicExample extends React.Component { _items = _items || createListItems(500); + this._onRenderItemColumn = this._onRenderItemColumn.bind(this); this._selection = new Selection({ onSelectionChanged: () => this.setState({ selectionDetails: this._getSelectionDetails() }) }); @@ -40,12 +42,27 @@ export class DetailsListBasicExample extends React.Component { onChanged={ text => this.setState({ items: text ? _items.filter(i => i.name.toLowerCase().indexOf(text) > -1) : _items }) } /> - + alert(`Item invoked: ${item.name}`) } + onRenderItemColumn={ this._onRenderItemColumn } + />
); } + private _onRenderItemColumn(item, index, column) { + if (column.key === 'name') { + return { item[column.key] }; + } + + return item[column.key]; + } + private _getSelectionDetails(): string { let selectionCount = this._selection.getSelectedCount(); @@ -53,7 +70,7 @@ export class DetailsListBasicExample extends React.Component { case 0: return 'No items selected'; case 1: - return '1 item selected: ' + (this._selection.getItems()[0] as any).name; + return '1 item selected: ' + (this._selection.getSelection()[0] as any).name; default: return `${ selectionCount } items selected`; } diff --git a/src/demo/pages/SelectionPage/SelectionPage.tsx b/src/demo/pages/SelectionPage/SelectionPage.tsx index 31663d16513b6..3a8fe86954bc9 100644 --- a/src/demo/pages/SelectionPage/SelectionPage.tsx +++ b/src/demo/pages/SelectionPage/SelectionPage.tsx @@ -16,11 +16,28 @@ export class SelectionPage extends React.Component { It exposes methods for accessing the selection state given an item index. If the items change, it can resolve the selection if items move in the array.

-

- SelectionZone is a React component that handles selection change events. - It can help abstract range selection, unselecting/selecting items based on selection modes, - and handling common keystrokes like ctrl-A for select all and escape to clear selection. + +

SelectionZone is a React component that acts as a mediator between the Selection object and elements. By providing it the Selection instance and rendering content within it, you can have it manage clicking/focus/keyboarding from the DOM and translate into selection updates. You just need to provide the right data-selection-* attributes on elements within each row/tile to give SelectionZone a hint what the intent is.

+ +

SelectionZone also takes in an onItemInvoked callback for when items are invoked. Invoking occurs when a user double clicks a row, presses enter while focused on it, or clicks within an element marked by the data-selection-invoke attribute.

+ +

Available attributes:

+
    +
  • + data-selection-index: the index of the item being represented.This would go on the root of the tile/row. +
    • +
  • + data-selection-invoke: this boolean flag would be set on the element which should immediately invoke the item on click.There is also a nuanced behavior where we will clear selection and select the item if mousedown occurs on an unselected item. +
    • +
  • + data-selection-toggle: this boolean flag would be set on the element which should handle toggles.This could be a checkbox or a div. +
    • +
  • + data-selection-toggle-all: this boolean flag indicates that clicking it should toggle all selection. +
    • +
+

Examples

diff --git a/src/demo/pages/SelectionPage/examples/Selection.Basic.Example.tsx b/src/demo/pages/SelectionPage/examples/Selection.Basic.Example.tsx index c45961e30e081..8d40138849a2d 100644 --- a/src/demo/pages/SelectionPage/examples/Selection.Basic.Example.tsx +++ b/src/demo/pages/SelectionPage/examples/Selection.Basic.Example.tsx @@ -64,7 +64,10 @@ export class SelectionBasicExample extends React.Component - + alert('item invoked: ' + item.name) }> { items.map((item, index) => ( { (selectionMode !== SelectionMode.none) && ( - +
) } { item.name } diff --git a/src/demo/pages/SelectionPage/examples/Selection.Example.scss b/src/demo/pages/SelectionPage/examples/Selection.Example.scss index 98b72f95a43b7..b370354beb6f8 100644 --- a/src/demo/pages/SelectionPage/examples/Selection.Example.scss +++ b/src/demo/pages/SelectionPage/examples/Selection.Example.scss @@ -10,6 +10,10 @@ border: none; } +.ms-SelectionItemExample:hover { + background: #EEE; +} + .ms-SelectionItemExample-name { display: inline-block; overflow: hidden; diff --git a/src/utilities/selection/Selection.ts b/src/utilities/selection/Selection.ts index ef070bccb877b..d78d309613b2d 100644 --- a/src/utilities/selection/Selection.ts +++ b/src/utilities/selection/Selection.ts @@ -175,6 +175,11 @@ export class Selection implements ISelection { // Clamp the index. index = Math.min(Math.max(0, index), this._items.length - 1); + // No-op on out of bounds selections. + if (index < 0 || index >= this._items.length) { + return; + } + let isExempt = this._exemptedIndices[index]; let hasChanged = false; let canSelect = !this._unselectableIndices[index]; @@ -210,17 +215,21 @@ export class Selection implements ISelection { } } - public selectToKey(key: string) { - this.selectToIndex(this._keyToIndexMap[key]); + public selectToKey(key: string, clearSelection?: boolean) { + this.selectToIndex(this._keyToIndexMap[key], clearSelection); } - public selectToIndex(index: number) { + public selectToIndex(index: number, clearSelection?: boolean) { let anchorIndex = this._anchoredIndex || 0; let startIndex = Math.min(index, anchorIndex); let endIndex = Math.max(index, anchorIndex); this.setChangeEvents(false); + if (clearSelection) { + this.setAllSelected(false); + } + for (; startIndex <= endIndex; startIndex++) { this.setIndexSelected(startIndex, true, false); } diff --git a/src/utilities/selection/SelectionZone.test.tsx b/src/utilities/selection/SelectionZone.test.tsx new file mode 100644 index 0000000000000..86e4cc7fdb70b --- /dev/null +++ b/src/utilities/selection/SelectionZone.test.tsx @@ -0,0 +1,224 @@ +/* tslint:disable:no-unused-variable */ +import * as React from 'react'; +/* tslint:enable:no-unused-variable */ + +import * as ReactDOM from 'react-dom'; +import * as ReactTestUtils from 'react-addons-test-utils'; +let { assert, expect } = chai; + +import { SelectionZone, Selection, SelectionMode } from './index'; +import { KeyCodes } from '../KeyCodes'; + +let _selection: Selection; +let _selectionZone: any; +let _componentElement: Element; +let _toggleAll: Element; +let _surface0: Element; +let _invoke0: Element; +let _toggle0: Element; +let _surface1: Element; +let _invoke1: Element; +let _toggle1: Element; +let _invoke2: Element; +let _toggle2: Element; +let _surface3: Element; + +let _onItemInvokeCalled: number; +let _lastItemInvoked: any; + +function _initializeSelection(selectionMode = SelectionMode.multiple) { + _selection = new Selection(); + _selection.setItems([{ key: 'a', }, { key: 'b' }, { key: 'c' }, { key: 'd' }]); + _selectionZone = ReactTestUtils.renderIntoDocument( + { _onItemInvokeCalled++; _lastItemInvoked = item; } }> + + + +
+ + +
+ +
+ + +
+ +
+ +
+ +
+ + + ); + + _componentElement = ReactDOM.findDOMNode(_selectionZone); + _toggleAll = _componentElement.querySelector('#toggleAll'); + _surface0 = _componentElement.querySelector('#surface0'); + _invoke0 = _componentElement.querySelector('#invoke0'); + _toggle0 = _componentElement.querySelector('#toggle0'); + _surface1 = _componentElement.querySelector('#surface1'); + _invoke1 = _componentElement.querySelector('#invoke1'); + _toggle1 = _componentElement.querySelector('#toggle1'); + _invoke2 = _componentElement.querySelector('#invoke2'); + _toggle2 = _componentElement.querySelector('#toggle2'); + _surface3 = _componentElement.querySelector('#surface3'); + + _onItemInvokeCalled = 0; + _lastItemInvoked = undefined; +} + +describe('SelectionZone', () => { + beforeEach(() => _initializeSelection()); + + it('toggles an item on click of toggle element', () => { + _simulateClick(_toggle0); + assert(_selection.isIndexSelected(0) === true, 'Index 0 not selected'); + _simulateClick(_toggle0); + assert(_selection.isIndexSelected(0) === false, 'Index 0 selected'); + assert(_onItemInvokeCalled === 0, 'onItemInvoked was called'); + }); + + it('toggles an item on dblclick of toggle element', () => { + ReactTestUtils.Simulate.doubleClick(_toggle0); + assert(_selection.isIndexSelected(0) === false, 'Index 0 selected'); + assert(_onItemInvokeCalled === 0, 'onItemInvoked was called'); + }); + + it('does not toggle an item on mousedown of toggle element', () => { + ReactTestUtils.Simulate.mouseDown(_toggle0); + assert(_selection.isIndexSelected(0) === false, 'Index 0 selected'); + assert(_onItemInvokeCalled === 0, 'onItemInvoked was called'); + }); + + it('selects an unselected item on mousedown of invoke without modifiers pressed', () => { + _selection.setAllSelected(true); + _selection.setIndexSelected(0, false, true); + + // Mousedown on the only unselected item's invoke surface should deselect all and select that one. + ReactTestUtils.Simulate.mouseDown(_invoke0); + expect(_selection.isIndexSelected(0)).equals(true, 'Index 0 not selected after mousedown'); + expect(_selection.getSelectedCount()).equals(1, 'Only 1 item should be selected'); + }); + + it('does nothing with mousedown of invoke when item is selected already', () => { + // Mousedown on an item that's already selected should do nothing. + _selection.setAllSelected(true); + ReactTestUtils.Simulate.mouseDown(_invoke0); + expect(_selection.isAllSelected()).equals(true, 'Expecting all items to be selected'); + }); + + it('calls the invoke callback on click of invoke area', () => { + _simulateClick(_invoke0); + assert(_onItemInvokeCalled === 1, 'onItemInvoked was not called 1 time after normal click'); + }); + + it('selects an unselected item on click of item surface element', () => { + _simulateClick(_surface0); + assert(_selection.isIndexSelected(0) === true, 'Index 0 not selected'); + assert(_onItemInvokeCalled === 0, 'onItemInvoked was called'); + }); + + it('does not unselect a selected item on click of item surface element', () => { + _selection.setIndexSelected(0, true, true); + _simulateClick(_surface0); + assert(_selection.isIndexSelected(0) === true, 'Index 0 not selected'); + assert(_onItemInvokeCalled === 0, 'onItemInvoked was called'); + }); + + it('does not select an unselected item on mousedown of item surface element', () => { + ReactTestUtils.Simulate.mouseDown(_surface0); + assert(_selection.isIndexSelected(0) === false, 'Index 0 selected'); + }); + + it('invokes an item on double clicking the surface element', () => { + ReactTestUtils.Simulate.doubleClick(_surface0); + assert(_onItemInvokeCalled === 1, 'Item was invoked'); + assert(_lastItemInvoked.key === 'a', 'Item invoked was not expected item'); + }); + + it('toggles all on toggle-all clicks', () => { + _simulateClick(_toggleAll); + expect(_selection.getSelectedCount()).equals(4, 'There were not 4 selected items'); + + _simulateClick(_toggle1); + expect(_selection.getSelectedCount()).equals(3, 'There were not 3 selected items after toggling index 1'); + + _simulateClick(_toggleAll); + expect(_selection.getSelectedCount()).equals(4, 'There were not 4 selected items after selecting all again'); + + _simulateClick(_toggleAll); + expect(_selection.getSelectedCount()).equals(0, 'There were not 0 selected items'); + }); + + it('suports mouse shift click range select scenarios', () => { + _simulateClick(_surface1); + expect(_selection.getSelectedCount()).equals(1, 'Clicked surface 1'); + + _simulateClick(_surface3, { shiftKey: true }); + expect(_selection.getSelectedCount()).equals(3, 'After clicking surface 1 and then shift clicking to surface 3'); + + _simulateClick(_surface0, { shiftKey: true }); + expect(_selection.getSelectedCount()).equals(2, 'After shift clicking surface 0'); + }); + + it('toggles by ctrl clicking a surface', () => { + _simulateClick(_toggleAll); + assert(_selection.getSelectedCount() === 4, 'There were not 4 selected items'); + + _simulateClick(_surface1, { + ctrlKey: true + }); + assert(_selection.getSelectedCount() === 3, 'There were not 3 selected items'); + }); + + it ('selects all on ctrl-a', () => { + ReactTestUtils.Simulate.keyDown(_componentElement, { ctrlKey: true, which: KeyCodes.a }); + expect(_selection.isAllSelected()).equals(true, 'Expecting that all is selected aftr ctrl-a'); + }); + + it('unselects all on escape', () => { + _selection.setAllSelected(true); + ReactTestUtils.Simulate.keyDown(_componentElement, { which: KeyCodes.escape }); + expect(_selection.getSelectedCount()).equals(0, 'Expecting that none is selected aftr escape'); + }); + + it('selects item on focus', () => { + ReactTestUtils.Simulate.focus(_surface0); + expect(_selection.isIndexSelected(0)).equals(true, 'Item 0 was not selected'); + }); + + it('does not select an item on focus if ctrl/meta is pressed', () => { + ReactTestUtils.Simulate.keyDown(_componentElement, { ctrlKey: true }); + ReactTestUtils.Simulate.focus(_surface0); + expect(_selection.isIndexSelected(0)).equals(false, 'Item 0 was selected on focus with modifier'); + }); + + it('does not select an item on focus when ignoreNextFocus is called', () => { + _selectionZone.ignoreNextFocus(); + ReactTestUtils.Simulate.focus(_surface0); + expect(_selection.isIndexSelected(0)).equals(false, 'Item 0 was selected on ignored focus'); + }); + + it('toggles an item when pressing space', () => { + ReactTestUtils.Simulate.keyDown(_surface0, { which: KeyCodes.space }); + expect(_selection.isIndexSelected(0)).equals(true, 'Expecting index 0 to become selected'); + ReactTestUtils.Simulate.keyDown(_surface0, { which: KeyCodes.space }); + expect(_selection.isIndexSelected(0)).equals(false, 'Expecting index 0 to become unselected'); + }); + + it('does not select the row when clicking on a toggle within an invoke element', () => { + ReactTestUtils.Simulate.mouseDown(_toggle2); + expect(_selection.isIndexSelected(2)).equals(false, 'Item 2 should have been unselected'); + }); +}); + +function _simulateClick(el, eventData?: React.SyntheticEventData) { + ReactTestUtils.Simulate.mouseDown(el, eventData); + ReactTestUtils.Simulate.focus(el, eventData); + ReactTestUtils.Simulate.click(el, eventData); +} diff --git a/src/utilities/selection/SelectionZone.tsx b/src/utilities/selection/SelectionZone.tsx index 55d6219940721..4d9e872fd8670 100644 --- a/src/utilities/selection/SelectionZone.tsx +++ b/src/utilities/selection/SelectionZone.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { EventGroup } from '../eventGroup/EventGroup'; +import { BaseComponent } from '../../common/BaseComponent'; import { SelectionLayout } from './SelectionLayout'; import { KeyCodes } from '../KeyCodes'; import { @@ -37,7 +37,7 @@ export interface ISelectionZoneProps extends React.Props { onItemInvoked?: (item?: any, index?: number, ev?: Event) => void; } -export class SelectionZone extends React.Component { +export class SelectionZone extends BaseComponent { public static defaultProps = { layout: new SelectionLayout(SelectionDirection.vertical), isMultiSelectEnabled: true, @@ -49,43 +49,29 @@ export class SelectionZone extends React.Component { root: HTMLElement }; - private _events: EventGroup; - private _isCtrlPressed: boolean; private _isShiftPressed: boolean; private _isMetaPressed: boolean; - private _hasClickedOnItem: boolean; private _shouldIgnoreFocus: boolean; - constructor() { - super(); - - this._events = new EventGroup(this); + constructor(props: ISelectionZoneProps) { + super(props); // Specifically for the click methods, we will want to use React eventing to allow // React and non React events to stop propagation and avoid the default SelectionZone // behaviors (like executing onInvoked.) + this._onFocus = this._onFocus.bind(this); + this._onKeyDown = this._onKeyDown.bind(this); this._onClick = this._onClick.bind(this); + this._onMouseDown = this._onMouseDown.bind(this); this._onDoubleClick = this._onDoubleClick.bind(this); + this._updateModifiers = this._updateModifiers.bind(this); + this.ignoreNextFocus = this.ignoreNextFocus.bind(this); } public componentDidMount() { - let element = this.refs.root; - - this._events.onAll(element, { - 'keydown': this._onKeyDown, - 'mousedown': this._onMouseDown - }); - - // Always know what the state of shift/ctrl/meta are. - this._events.on(element, 'focus', this._onFocus, true); - this._events.on(window, 'keydown', this._onKeyChangeCapture, true); - this._events.on(window, 'keyup', this._onKeyChangeCapture, true); - - } - - public componentWillUnmount() { - this._events.dispose(); + // Track the latest modifier keys globally. + this._events.on(window, 'keydown keyup', this._updateModifiers); } public render() { @@ -93,6 +79,10 @@ export class SelectionZone extends React.Component {
@@ -111,161 +101,287 @@ export class SelectionZone extends React.Component { this._shouldIgnoreFocus = true; } - private _onFocus(ev: FocusEvent) { - if (this._shouldIgnoreFocus) { + /** + * When we focus an item, for single/multi select scenarios, we should try to select it immediately + * as long as the focus did not originate from a mouse down/touch event. For those cases, we handle them + * specially. + */ + private _onFocus(ev: React.FocusEvent) { + let target = ev.target as HTMLElement; + let { selection, selectionMode } = this.props; + let isToggleModifierPressed = this._isCtrlPressed || this._isMetaPressed; + + if (this._shouldIgnoreFocus || selectionMode === SelectionMode.none) { this._shouldIgnoreFocus = false; return; } - let { selection, selectionMode } = this.props; - let index = this._getIndexFromElement(ev.target as HTMLElement); + let isToggle = this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME); + let itemRoot = this._findItemRoot(target); - if (index >= 0 && selectionMode !== SelectionMode.none && !this._hasClickedOnItem) { - selection.setChangeEvents(false); + if (!isToggle && itemRoot) { + let index = this._getItemIndex(itemRoot); + + if (isToggleModifierPressed) { + // set anchor only. + selection.setIndexSelected(index, selection.isIndexSelected(index), true); + } else { + this._onItemSurfaceClick(ev, index); + } + } + } - if (this._isShiftPressed && selectionMode === SelectionMode.multiple) { - if (!this._isCtrlPressed && !this._isMetaPressed) { - selection.setAllSelected(false); + private _onMouseDown(ev: React.MouseEvent) { + this._updateModifiers(ev); + + let target = ev.target as HTMLElement; + let itemRoot = this._findItemRoot(target); + + while (target !== this.refs.root) { + if (this._hasAttribute(target, SELECTALL_TOGGLE_ALL_ATTRIBUTE_NAME)) { + break; + } else if (itemRoot) { + if (this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME)) { + break; + } else if (this._hasAttribute(target, SELECTION_INVOKE_ATTRIBUTE_NAME)) { + this._onInvokeMouseDown(ev, this._getItemIndex(itemRoot)); + break; + } else if (target === itemRoot) { + break; } - selection.selectToIndex(index); - } else if (!this._isCtrlPressed && !this._isMetaPressed) { - selection.setAllSelected(false); - selection.setIndexSelected(index, true, true); } - selection.setChangeEvents(true); + target = target.parentElement; } - - this._hasClickedOnItem = false; } - private _onMouseDown(ev: MouseEvent) { - // We need to reset the key states for ctrl/meta/etc. - this._onKeyChangeCapture(ev as any); + private _onClick(ev: React.MouseEvent) { + this._updateModifiers(ev); + + let target = ev.target as HTMLElement; + let itemRoot = this._findItemRoot(target); + + while (target !== this.refs.root) { + if (this._hasAttribute(target, SELECTALL_TOGGLE_ALL_ATTRIBUTE_NAME)) { + this._onToggleAllClick(ev); + break; + } else if (itemRoot) { + let index = this._getItemIndex(itemRoot); + + if (this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME)) { + if (this._isShiftPressed) { + this._onItemSurfaceClick(ev, index); + } else { + this._onToggleClick(ev, index); + } + break; + } else if (this._hasAttribute(target, SELECTION_INVOKE_ATTRIBUTE_NAME)) { + this._onInvokeClick(ev, index); + break; + } else if (target === itemRoot) { + this._onItemSurfaceClick(ev, index); + break; + } + } + + target = target.parentElement; + } + } + /** + * In multi selection, if you double click within an item's root (but not within the invoke element or input elements), + * we should execute the invoke handler. + */ + private _onDoubleClick(ev: React.MouseEvent) { let target = ev.target as HTMLElement; - let { selectionMode } = this.props; - let index = this._getIndexFromElement(target, true); + let { selectionMode, onItemInvoked } = this.props; + let itemRoot = this._findItemRoot(target); + + if (itemRoot && onItemInvoked && selectionMode !== SelectionMode.none && !this._isInputElement(target)) { + let index = this._getItemIndex(itemRoot); + + while (target !== this.refs.root) { + if ( + this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME) || + this._hasAttribute(target, SELECTION_INVOKE_ATTRIBUTE_NAME)) { + break; + } else if (target === itemRoot) { + this._onInvokeClick(ev, index); + break; + } + + target = target.parentElement; + } - if (index >= 0 && selectionMode !== SelectionMode.none) { - this._hasClickedOnItem = true; + target = target.parentElement; } } - private _onClick(ev: React.MouseEvent) { + private _onKeyDown(ev: React.KeyboardEvent) { + this._updateModifiers(ev); + let target = ev.target as HTMLElement; - let { selection, selectionMode, onItemInvoked } = this.props; - let isToggleElement = this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME) || ev.ctrlKey || ev.metaKey; - let index = this._getIndexFromElement(target, true); + let { selection, selectionMode } = this.props; + let isSelectAllKey = ev.which === KeyCodes.a && (this._isCtrlPressed || this._isMetaPressed); + let isClearSelectionKey = ev.which === KeyCodes.escape; - if (index >= 0 && selectionMode !== SelectionMode.none) { - let isSelected = selection.isIndexSelected(index); + // Ignore key downs from input elements. + if (this._isInputElement(target)) { + return; + } - // Disable change events. - selection.setChangeEvents(false); + // If ctrl-a is pressed, select all (if all are not already selected.) + if (isSelectAllKey && selectionMode === SelectionMode.multiple && !selection.isAllSelected()) { + selection.setAllSelected(true); + ev.stopPropagation(); + ev.preventDefault(); + return; + } - let isInvokable = this._hasAttribute(target, SELECTION_INVOKE_ATTRIBUTE_NAME); + // If escape is pressed, clear selection (if any are selected.) + if (isClearSelectionKey && selection.getSelectedCount() > 0) { + selection.setAllSelected(false); + ev.stopPropagation(); + ev.preventDefault(); + return; + } - if (isInvokable && onItemInvoked) { - onItemInvoked(selection.getItems()[index], index, ev.nativeEvent); - } else if (ev.shiftKey && selectionMode === SelectionMode.multiple) { - if (!ev.ctrlKey && !ev.metaKey) { - selection.setAllSelected(false); - } - selection.selectToIndex(index); - } else { - if (selectionMode === SelectionMode.single || !isToggleElement) { - selection.setAllSelected(false); + let itemRoot = this._findItemRoot(target); + + // If a key was pressed within an item, we should treat "enters" as invokes and "space" as toggle + if (itemRoot) { + let index = this._getItemIndex(itemRoot); + + while (target !== this.refs.root) { + if (this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME)) { + // For toggle elements, assuming they are rendered as buttons, they will generate a click event, + // so we can no-op for any keydowns in this case. + break; + } else if (target === itemRoot) { + if (ev.which === KeyCodes.enter) { + this._onInvokeClick(ev, index); + } else if (ev.which === KeyCodes.space) { + this._onToggleClick(ev, index); + } + break; } - selection.setIndexSelected(index, isToggleElement ? !isSelected : true, !ev.shiftKey); + target = target.parentElement; } + } + } - // Re-enabled change events. - selection.setChangeEvents(true); - } else if (onItemInvoked) { - onItemInvoked(selection.getItems()[index], index, ev.nativeEvent); + private _onToggleAllClick(ev: React.MouseEvent) { + let { selection, selectionMode } = this.props; + + if (selectionMode === SelectionMode.multiple) { + selection.toggleAllSelected(); + ev.stopPropagation(); + ev.preventDefault(); } } - private _onDoubleClick(ev: React.MouseEvent) { - let target = ev.target as HTMLElement; - let isToggleElement = this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME) || ev.ctrlKey || ev.metaKey; + private _onToggleClick(ev: React.MouseEvent | React.KeyboardEvent, index: number) { + let { selection, selectionMode } = this.props; - if (isToggleElement) { + if (selectionMode === SelectionMode.multiple) { + selection.toggleIndexSelected(index); + } else if (selectionMode === SelectionMode.single) { + let isSelected = selection.isIndexSelected(index); + selection.setChangeEvents(false); + selection.setAllSelected(false); + selection.setIndexSelected(index, !isSelected, true); + } else { return; } - let { onItemInvoked, selection } = this.props; - let index = this._getIndexFromElement(target, true); + ev.stopPropagation(); + ev.preventDefault(); + } + + private _onInvokeClick(ev: React.MouseEvent | React.KeyboardEvent, index: number) { + let { selection, onItemInvoked } = this.props; - if (onItemInvoked && index >= 0) { + if (onItemInvoked) { onItemInvoked(selection.getItems()[index], index, ev.nativeEvent); + ev.preventDefault(); + ev.stopPropagation(); + } + } + + private _onItemSurfaceClick(ev: React.SyntheticEvent, index: number) { + let { selection, selectionMode } = this.props; + let isToggleModifierPressed = this._isCtrlPressed || this._isMetaPressed; + + if (selectionMode === SelectionMode.multiple) { + if (this._isShiftPressed) { + selection.selectToIndex(index, !isToggleModifierPressed); + } else if (isToggleModifierPressed) { + selection.toggleIndexSelected(index); + } else { + this._clearAndSelectIndex(index); + } + } else if (selectionMode === SelectionMode.single) { + this._clearAndSelectIndex(index); + } + } + + private _onInvokeMouseDown(ev: React.MouseEvent | React.KeyboardEvent, index: number) { + let { selection } = this.props; + + // Only do work if item is not selected. + if (selection.isIndexSelected(index)) { + return; + } + + this._clearAndSelectIndex(index); + } + + private _clearAndSelectIndex(index: number) { + let { selection } = this.props; + let isAlreadySingleSelected = selection.getSelectedCount() === 1 && selection.isIndexSelected(index); + + if (!isAlreadySingleSelected) { + selection.setChangeEvents(false); + selection.setAllSelected(false); + selection.setIndexSelected(index, true, true); + selection.setChangeEvents(true); } } - private _onKeyChangeCapture(ev: KeyboardEvent) { + /** + * We need to track the modifier key states so that when focus events occur, which do not contain + * modifier states in the Event object, we know how to behave. + */ + private _updateModifiers(ev: React.KeyboardEvent | React.MouseEvent) { this._isShiftPressed = ev.shiftKey; this._isCtrlPressed = ev.ctrlKey; this._isMetaPressed = ev.metaKey; } - private _onKeyDown(ev: KeyboardEvent) { - let target = ev.target as HTMLElement; - let { selection, selectionMode, onItemInvoked } = this.props; - let isToggleElement = this._hasAttribute(target, SELECTION_TOGGLE_ATTRIBUTE_NAME); - let isToggleAllElement = !isToggleElement && this._hasAttribute(target, SELECTALL_TOGGLE_ALL_ATTRIBUTE_NAME); - let index = this._getIndexFromElement(target, true); + private _findItemRoot(target: HTMLElement): HTMLElement { + let { selection } = this.props; - if (index >= 0 && !this._isInputElement(target) && selectionMode !== SelectionMode.none) { - let isSelected = selection.isIndexSelected(index); + while (target !== this.refs.root) { + let indexValue = target.getAttribute(SELECTION_INDEX_ATTRIBUTE_NAME); + let index = Number(indexValue); - if (ev.which === KeyCodes.space) { - if (isToggleAllElement) { - if (selectionMode === SelectionMode.multiple) { - selection.toggleAllSelected(); - } - } else { // an item - selection.setChangeEvents(false); - if (selectionMode === SelectionMode.single) { - selection.setAllSelected(false); - } - selection.setIndexSelected(index, !isSelected, true); - selection.setChangeEvents(true); - } - } else if (ev.which === KeyCodes.enter) { - if (isToggleAllElement) { - selection.toggleAllSelected(); - } else if (isToggleElement) { - selection.setChangeEvents(false); - if (selectionMode === SelectionMode.single) { - selection.setAllSelected(false); - } - selection.setIndexSelected(index, !isSelected, true); - selection.setChangeEvents(true); - } else if (this._getIndexFromElement(target) >= 0 && onItemInvoked) { - // if the target IS the item, and not a link inside, then call the invoke method. - onItemInvoked(selection.getItems()[index], index, ev); - } else { - return; - } - } else if (ev.which === KeyCodes.a && (ev.ctrlKey || ev.metaKey) && selectionMode === SelectionMode.multiple) { - selection.setAllSelected(true); - } else if (ev.which === KeyCodes.escape) { - if (selection.getSelectedCount() > 0) { - selection.setAllSelected(false); - } else { - return; - } - } else { - return; + if (indexValue !== null && index >= 0 && index < selection.getItems().length ) { + break; } - } else { - return; + + target = target.parentElement; } - ev.preventDefault(); - ev.stopPropagation(); + if (target === this.refs.root) { + return undefined; + } + + return target; + } + + private _getItemIndex(itemRoot: HTMLElement): number { + return Number(itemRoot.getAttribute(SELECTION_INDEX_ATTRIBUTE_NAME)); } private _hasAttribute(element: HTMLElement, attributeName: string) { @@ -283,22 +399,4 @@ export class SelectionZone extends React.Component { return element.tagName === 'INPUT' || element.tagName === 'TEXTAREA'; } - private _getIndexFromElement(element: HTMLElement, traverseParents?: boolean): number { - let index = -1; - - do { - let indexString = element.getAttribute(SELECTION_INDEX_ATTRIBUTE_NAME); - - if (indexString) { - index = Number(indexString); - break; - } - if (element !== this.refs.root) { - element = element.parentElement; - } - } while (traverseParents && element !== this.refs.root); - - return index; - } - } diff --git a/src/utilities/selection/interfaces.ts b/src/utilities/selection/interfaces.ts index 5baf30f053352..bd94b7854ddf8 100644 --- a/src/utilities/selection/interfaces.ts +++ b/src/utilities/selection/interfaces.ts @@ -39,8 +39,8 @@ export interface ISelection { // Write range selection methods. - selectToKey(key: string); - selectToIndex(index: number); + selectToKey(key: string, clearSelection?: boolean); + selectToIndex(index: number, clearSelection?: boolean); // Toggle helpers. From 030b1c27754e9df17513edc447cf72e66a017816 Mon Sep 17 00:00:00 2001 From: David Zearing Date: Sun, 21 Aug 2016 19:46:02 -0700 Subject: [PATCH 07/11] v0.45.0 (#173) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 460a24acb857c..481ad2af6f513 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "office-ui-fabric-react", - "version": "0.44.0", + "version": "0.45.0", "description": "Reusable React components for building experiences for Office 365.", "main": "lib/index.js", "typings": "lib/index.d.ts", From 40d007ff3060c8f782c47c82d459b66b93eeac80 Mon Sep 17 00:00:00 2001 From: Yimin Wu Date: Mon, 22 Aug 2016 09:14:05 -0700 Subject: [PATCH 08/11] Make DocumentCardAction props extend from Button props. This will make sure we can set all the properties on the action button such as aria label. (#170) * Make DocumentCardAction props extend from Button props. This will make sure we can set all the properties on the action button such as aria label. * Add missing semicolon * remove IDocumentCardAction interface since we actually only need button props. --- .../DocumentCard/DocumentCard.Props.ts | 15 +----- .../DocumentCard/DocumentCardActions.tsx | 18 ++++--- .../DocumentCard.Complete.Example.tsx | 47 +++++++++++-------- 3 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/components/DocumentCard/DocumentCard.Props.ts b/src/components/DocumentCard/DocumentCard.Props.ts index ce204741f69af..22bd40bb8b61e 100644 --- a/src/components/DocumentCard/DocumentCard.Props.ts +++ b/src/components/DocumentCard/DocumentCard.Props.ts @@ -7,6 +7,7 @@ import { DocumentCardActivity } from './DocumentCardActivity'; import { DocumentCardActions } from './DocumentCardActions'; import { PersonaInitialsColor } from '../../Persona'; import { ImageFit } from '../../Image'; +import { IButtonProps } from '../../Button'; export interface IDocumentCardProps extends React.Props { /** @@ -145,22 +146,10 @@ export interface IDocumentCardActionsProps extends React.Props void; -} diff --git a/src/components/DocumentCard/DocumentCardActions.tsx b/src/components/DocumentCard/DocumentCardActions.tsx index 44ab8e2635540..8dbdcec9bc255 100644 --- a/src/components/DocumentCard/DocumentCardActions.tsx +++ b/src/components/DocumentCard/DocumentCardActions.tsx @@ -10,16 +10,14 @@ export class DocumentCardActions extends React.Component - { actions && actions.map((action, index) => ( -
-
- )) } + { actions && actions.map((action, index) => { + action.buttonType = ButtonType.icon; + return ( +
+
+ ); + }) } { views && (
diff --git a/src/demo/pages/DocumentCardPage/examples/DocumentCard.Complete.Example.tsx b/src/demo/pages/DocumentCardPage/examples/DocumentCard.Complete.Example.tsx index 163fd4e2188c9..10a86032caf31 100644 --- a/src/demo/pages/DocumentCardPage/examples/DocumentCard.Complete.Example.tsx +++ b/src/demo/pages/DocumentCardPage/examples/DocumentCard.Complete.Example.tsx @@ -62,28 +62,37 @@ export class DocumentCardCompleteExample extends React.Component { } /> { - console.log('You clicked the share action.'); - ev.preventDefault(); - ev.stopPropagation(); - } + actions={ + [ + { + icon: 'share', + onClick: (ev: any) => { + console.log('You clicked the share action.'); + ev.preventDefault(); + ev.stopPropagation(); }, - { icon: 'pinLeft', onClick: (ev: any) => { - console.log('You clicked the pin action.'); - ev.preventDefault(); - ev.stopPropagation(); - } + ariaLabel: 'share action' + }, + { + icon: 'pinLeft', + onClick: (ev: any) => { + console.log('You clicked the pin action.'); + ev.preventDefault(); + ev.stopPropagation(); }, - { icon: 'bell', onClick: (ev: any) => { - console.log('You clicked the bell action.'); - ev.preventDefault(); - ev.stopPropagation(); - } + ariaLabel: 'pin left action' + }, + { + icon: 'bell', + onClick: (ev: any) => { + console.log('You clicked the bell action.'); + ev.preventDefault(); + ev.stopPropagation(); }, - ] - } + ariaLabel: 'bell action' + }, + ] + } views={ 432 } /> From e0c1614df1914ef21e6d1c94e07be76caa24f810 Mon Sep 17 00:00:00 2001 From: Aniket Handa Date: Mon, 22 Aug 2016 12:40:32 -0700 Subject: [PATCH 09/11] Update .pullapprove.yml (#180) --- .pullapprove.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pullapprove.yml b/.pullapprove.yml index b113477b13f3d..f6a00340931cf 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -16,5 +16,6 @@ reviewers: - yiminwu - antonlabunets - cschlechty + - atneik name: pullapprove required: 1 From 9c9e3e0af222a87be595a0ee0c69a0957e9ec131 Mon Sep 17 00:00:00 2001 From: Aniket Handa Date: Tue, 23 Aug 2016 15:21:50 -0700 Subject: [PATCH 10/11] Fixing extra on left of contextualMenu icon and also size of right chevron --- .../ContextualMenu/ContextualMenu.scss | 16 +++++----------- src/components/ContextualMenu/ContextualMenu.tsx | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/components/ContextualMenu/ContextualMenu.scss b/src/components/ContextualMenu/ContextualMenu.scss index afb9ce6b0bf59..ba59d87e37421 100644 --- a/src/components/ContextualMenu/ContextualMenu.scss +++ b/src/components/ContextualMenu/ContextualMenu.scss @@ -4,7 +4,7 @@ $ContextualMenu-background: $ms-color-white; $ContextualMenu-itemHover: $ms-color-neutralLighter; $ContextualMenu-expandedItemBackground: $ms-color-neutralQuaternaryAlt; $ContextualMenu-itemHeight: 36px; -$ContextualMenu-iconWidth: 24px; +$ContextualMenu-iconWidth: 14px; .ms-ContextualMenu { background-color: $ContextualMenu-background; @@ -36,7 +36,7 @@ $ContextualMenu-iconWidth: 24px; background: none; border: none; margin: 0; - padding: 0 8px; + padding: 0 4px; min-width: 100%; height: $ContextualMenu-itemHeight; line-height: $ContextualMenu-itemHeight; @@ -68,23 +68,17 @@ $ContextualMenu-iconWidth: 24px; position: relative; } -.ms-ContextualMenu-checkmark { - display: inline-block; - min-height: 1px; - width: $ContextualMenu-iconWidth; - text-align: center; -} - .ms-ContextualMenu-icon { display: inline-block; min-height: 1px; width: $ContextualMenu-iconWidth; text-align: center; vertical-align: top; + padding: 0px 4px; } .ms-ContextualMenu-itemText { - padding: 0 4px; + @include padding(0, 12px, 0, 4px); } .ms-Icon.ms-ContextualMenu-submenuChevron { @@ -93,6 +87,6 @@ $ContextualMenu-iconWidth: 24px; line-height: $ContextualMenu-itemHeight; vertical-align: middle; @include right(8px); - width: 16px; text-align: center; + font-size: $ms-icon-size-xs; } \ No newline at end of file diff --git a/src/components/ContextualMenu/ContextualMenu.tsx b/src/components/ContextualMenu/ContextualMenu.tsx index a88c9129f059b..03b7c484f5f8d 100644 --- a/src/components/ContextualMenu/ContextualMenu.tsx +++ b/src/components/ContextualMenu/ContextualMenu.tsx @@ -249,7 +249,7 @@ export class ContextualMenu extends React.Component ) : (null) } From 847e40e68f70a9efa1c528bf8168263b5a53766d Mon Sep 17 00:00:00 2001 From: Aniket Handa Date: Tue, 23 Aug 2016 16:25:43 -0700 Subject: [PATCH 11/11] Charms cursor pointed; and 40px sqs --- .../ContextualMenuPage/examples/ContextualMenuExample.scss | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/demo/pages/ContextualMenuPage/examples/ContextualMenuExample.scss b/src/demo/pages/ContextualMenuPage/examples/ContextualMenuExample.scss index 5925a43031450..ed2b33ff5fd55 100644 --- a/src/demo/pages/ContextualMenuPage/examples/ContextualMenuExample.scss +++ b/src/demo/pages/ContextualMenuPage/examples/ContextualMenuExample.scss @@ -26,12 +26,13 @@ .ms-ContextualMenu-customizationExample-item { display: inline-block; - width: 39px; - height: 39px; - line-height: 39px; + width: 40px; + height: 40px; + line-height: 40px; text-align: center; vertical-align: middle; margin-bottom: 8px; + cursor: pointer; &:hover { background-color: #eaeaea;