From 3ec5432defe9ab1569c1266279c64efe32810b5f Mon Sep 17 00:00:00 2001 From: Lambert W Date: Thu, 22 Feb 2018 15:46:32 -0800 Subject: [PATCH 1/8] Made disabled buttons focusable --- .../ContextualMenu/ContextualMenu.test.tsx | 36 ++++++++++++--- .../ContextualMenu/ContextualMenu.tsx | 44 ++++++++++++------- .../examples/ContextualMenu.Basic.Example.tsx | 15 ++++--- 3 files changed, 67 insertions(+), 28 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx index 3efa04c9d2991c..c982e873b6247a 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -239,11 +239,15 @@ describe('ContextualMenu', () => { expect(document.querySelector('.SubMenuClass')).toBeDefined(); }); - it('applies disabled property when `disabled` is true', () => { + it('can focus on disabled items', () => { const items: IContextualMenuItem[] = [ { name: 'TestText 1', key: 'TestKey1', + }, + { + name: 'TestText 2', + key: 'TestKey2', disabled: true, }, ]; @@ -254,17 +258,30 @@ describe('ContextualMenu', () => { /> ); - const menuItem = document.querySelector('button.ms-ContextualMenu-link') as HTMLButtonElement; + const menuItems = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf; + + menuItems[0].focus(); + expect(document.activeElement.className.split(' ')).toContain('ms-ContextualMenu-link'); + expect(document.activeElement.className.split(' ')).not.toContain('is-disabled'); - expect(menuItem.disabled).toBeTruthy(); + menuItems[1].focus(); + expect(document.activeElement.className.split(' ')).toContain('is-disabled'); }); - it('applies disabled property when deprecated property `isDisabled` is true', () => { + it('cannot click on disabled items', () => { + let activeItemClicked = false; + let disabledItemClicked = false; const items: IContextualMenuItem[] = [ { name: 'TestText 1', key: 'TestKey1', - isDisabled: true, + onClick: () => activeItemClicked = true + }, + { + name: 'TestText 2', + key: 'TestKey2', + disabled: true, + onClick: () => disabledItemClicked = false }, ]; @@ -274,9 +291,14 @@ describe('ContextualMenu', () => { /> ); - const menuItem = document.querySelector('button.ms-ContextualMenu-link') as HTMLButtonElement; + const menuItem = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf; + expect(menuItem.length).toEqual(2); + + menuItem[0].click(); + expect(activeItemClicked).toEqual(true); - expect(menuItem.disabled).toBeTruthy(); + menuItem[1].click(); + expect(disabledItemClicked).toEqual(false); }); it('renders headers properly', () => { diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx index 7d9fe69b402b1b..e3824de8ed7eca 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx @@ -290,7 +290,7 @@ export class ContextualMenu extends BaseComponent { title } } { (items && items.length) ? ( ); } @@ -492,11 +492,11 @@ export class ContextualMenu extends BaseComponent ); @@ -531,6 +531,10 @@ export class ContextualMenu extends BaseComponent this._onItemMouseDown(item, ev), onMouseMove: this._onItemMouseMove.bind(this, item), - disabled: this._isItemDisabled(item), href: item.href, title: item.title, 'aria-label': ariaLabel, @@ -556,10 +559,16 @@ export class ContextualMenu extends BaseComponent - + ); } @@ -617,7 +626,7 @@ export class ContextualMenu extends BaseComponent, + , ); } @@ -640,7 +649,7 @@ export class ContextualMenu extends BaseComponent this._onItemMouseDown(item, ev), onMouseMove: this._onItemMouseMove.bind(this, item) }), - + ); } @@ -821,6 +830,9 @@ export class ContextualMenu extends BaseComponent) { + if (item.disabled) { + return; + } if (item.onClick) { item.onClick(ev, item); } else if (this.props.onItemClick) { diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/examples/ContextualMenu.Basic.Example.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/examples/ContextualMenu.Basic.Example.tsx index 4fa1b5f774ef5f..318cefb3872fde 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/examples/ContextualMenu.Basic.Example.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/examples/ContextualMenu.Basic.Example.tsx @@ -23,7 +23,8 @@ export class ContextualMenuBasicExample extends React.Component { items: [ { key: 'newItem', - name: 'New' + name: 'New', + onClick: () => console.log('New clicked') }, { key: 'divider_1', @@ -31,20 +32,24 @@ export class ContextualMenuBasicExample extends React.Component { }, { key: 'rename', - name: 'Rename' + name: 'Rename', + onClick: () => console.log('Rename clicked') }, { key: 'edit', - name: 'Edit' + name: 'Edit', + onClick: () => console.log('Edit clicked') }, { key: 'properties', - name: 'Properties' + name: 'Properties', + onClick: () => console.log('Properties clicked') }, { key: 'disabled', name: 'Disabled item', - disabled: true + disabled: true, + onClick: () => console.error('Disabled item should not be clickable.') } ] } } From 01ae88046b8de040cc36162fd4ab0525b5716222 Mon Sep 17 00:00:00 2001 From: Lambert W Date: Thu, 22 Feb 2018 15:49:05 -0800 Subject: [PATCH 2/8] Change file --- ...ellan-focusableDisabledItems_2018-02-22-23-48.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/office-ui-fabric-react/magellan-focusableDisabledItems_2018-02-22-23-48.json diff --git a/common/changes/office-ui-fabric-react/magellan-focusableDisabledItems_2018-02-22-23-48.json b/common/changes/office-ui-fabric-react/magellan-focusableDisabledItems_2018-02-22-23-48.json new file mode 100644 index 00000000000000..43df507df1227e --- /dev/null +++ b/common/changes/office-ui-fabric-react/magellan-focusableDisabledItems_2018-02-22-23-48.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "[ContextualMenu] Disabled buttons are focusable", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "law@microsoft.com" +} \ No newline at end of file From e99090beff8739c2097c08baf036ea2fd6769261 Mon Sep 17 00:00:00 2001 From: Lambert W Date: Fri, 23 Feb 2018 11:12:06 -0800 Subject: [PATCH 3/8] Fixed test logic --- .../src/components/ContextualMenu/ContextualMenu.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx index c982e873b6247a..a351158ccc0de8 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -281,7 +281,7 @@ describe('ContextualMenu', () => { name: 'TestText 2', key: 'TestKey2', disabled: true, - onClick: () => disabledItemClicked = false + onClick: () => disabledItemClicked = true }, ]; From a1c0164a707ff0280605565c99d5dec937aaad69 Mon Sep 17 00:00:00 2001 From: Lambert W Date: Fri, 23 Feb 2018 11:19:16 -0800 Subject: [PATCH 4/8] Added test failure --- .../src/components/ContextualMenu/ContextualMenu.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx index a351158ccc0de8..4526be5ebe6e96 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -281,7 +281,10 @@ describe('ContextualMenu', () => { name: 'TestText 2', key: 'TestKey2', disabled: true, - onClick: () => disabledItemClicked = true + onClick: () => { + disabledItemClicked = true; + fail('Disabled item should not be clickable'); + } }, ]; From 8ae04caed45bbb938d75670c22070942c12baea6 Mon Sep 17 00:00:00 2001 From: Lambert W Date: Fri, 23 Feb 2018 11:36:41 -0800 Subject: [PATCH 5/8] Added isDisabled tests --- .../ContextualMenu/ContextualMenu.test.tsx | 48 ++++++++++++++----- .../ContextualMenu/ContextualMenu.tsx | 2 +- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx index 4526be5ebe6e96..38ef4cec87f0de 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -250,6 +250,11 @@ describe('ContextualMenu', () => { key: 'TestKey2', disabled: true, }, + { + name: 'TestText 3', + key: 'TestKey3', + isDisabled: true, + }, ]; ReactTestUtils.renderIntoDocument( @@ -259,30 +264,48 @@ describe('ContextualMenu', () => { ); const menuItems = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf; + expect(menuItems.length).toEqual(3); menuItems[0].focus(); - expect(document.activeElement.className.split(' ')).toContain('ms-ContextualMenu-link'); + expect(document.activeElement.textContent).toEqual('TestText 1'); expect(document.activeElement.className.split(' ')).not.toContain('is-disabled'); menuItems[1].focus(); + expect(document.activeElement.textContent).toEqual('TestText 2'); + expect(document.activeElement.className.split(' ')).toContain('is-disabled'); + + menuItems[2].focus(); + expect(document.activeElement.textContent).toEqual('TestText 3'); expect(document.activeElement.className.split(' ')).toContain('is-disabled'); }); it('cannot click on disabled items', () => { - let activeItemClicked = false; - let disabledItemClicked = false; + let itemsClicked = [ + false, + false, + false + ]; const items: IContextualMenuItem[] = [ { name: 'TestText 1', key: 'TestKey1', - onClick: () => activeItemClicked = true + onClick: () => itemsClicked[0] = true }, { name: 'TestText 2', key: 'TestKey2', disabled: true, onClick: () => { - disabledItemClicked = true; + itemsClicked[1] = true; + fail('Disabled item should not be clickable'); + } + }, + { + name: 'TestText 3', + key: 'TestKey3', + isDisabled: true, + onClick: () => { + itemsClicked[2] = true; fail('Disabled item should not be clickable'); } }, @@ -294,14 +317,17 @@ describe('ContextualMenu', () => { /> ); - const menuItem = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf; - expect(menuItem.length).toEqual(2); + const menuItems = document.querySelectorAll('button.ms-ContextualMenu-link') as NodeListOf; + expect(menuItems.length).toEqual(3); + + menuItems[0].click(); + expect(itemsClicked[0]).toEqual(true); - menuItem[0].click(); - expect(activeItemClicked).toEqual(true); + menuItems[1].click(); + expect(itemsClicked[1]).toEqual(false); - menuItem[1].click(); - expect(disabledItemClicked).toEqual(false); + menuItems[2].click(); + expect(itemsClicked[2]).toEqual(false); }); it('renders headers properly', () => { diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx index e3824de8ed7eca..8b48c5ae2b3a1b 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx @@ -830,7 +830,7 @@ export class ContextualMenu extends BaseComponent) { - if (item.disabled) { + if (item.disabled || item.isDisabled) { return; } if (item.onClick) { From 3c9f4e2b8bb3e7234651370d61525c124d3f5b56 Mon Sep 17 00:00:00 2001 From: Lambert W Date: Fri, 23 Feb 2018 11:59:54 -0800 Subject: [PATCH 6/8] Fixed tslint in test --- .../src/components/ContextualMenu/ContextualMenu.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx index 38ef4cec87f0de..c9861234dc780d 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -280,7 +280,7 @@ describe('ContextualMenu', () => { }); it('cannot click on disabled items', () => { - let itemsClicked = [ + const itemsClicked = [ false, false, false From dec1a07c15039854f56f01fae64bb8ef4b29b60a Mon Sep 17 00:00:00 2001 From: Lambert W Date: Fri, 23 Feb 2018 13:42:57 -0800 Subject: [PATCH 7/8] Updated DetailsList snapshot --- .../DetailsList/DetailsList.test.tsx | 1 - .../__snapshots__/DetailsList.test.tsx.snap | 59 ++++++++++++++++++- scripts/tasks/webpack-resources.js | 1 + 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/DetailsList/DetailsList.test.tsx b/packages/office-ui-fabric-react/src/components/DetailsList/DetailsList.test.tsx index ff04576a4f4e89..a02f86a9dc39c3 100644 --- a/packages/office-ui-fabric-react/src/components/DetailsList/DetailsList.test.tsx +++ b/packages/office-ui-fabric-react/src/components/DetailsList/DetailsList.test.tsx @@ -74,7 +74,6 @@ describe('DetailsList', () => { if (value === null || value === undefined) { value = ''; } - console.log('Rendered column'); return (
{ value } diff --git a/packages/office-ui-fabric-react/src/components/DetailsList/__snapshots__/DetailsList.test.tsx.snap b/packages/office-ui-fabric-react/src/components/DetailsList/__snapshots__/DetailsList.test.tsx.snap index 3781b2dbc69d23..d4d66656bcecad 100644 --- a/packages/office-ui-fabric-react/src/components/DetailsList/__snapshots__/DetailsList.test.tsx.snap +++ b/packages/office-ui-fabric-react/src/components/DetailsList/__snapshots__/DetailsList.test.tsx.snap @@ -62,16 +62,52 @@ exports[`DetailsList renders List correctly 1`] = ` role="checkbox" >
@@ -79,10 +115,27 @@ exports[`DetailsList renders List correctly 1`] = ` aria-hidden={true} className= ms-Check-check - undefined { display: inline-block; } + { + color: #c8c8c8; + font-size: 16px; + height: 18px; + left: .5px; + opacity: 0; + position: absolute; + text-align: center; + top: 0px; + vertical-align: middle; + width: 18px; + } + &:hover { + opacity: 1; + } + @media screen and (-ms-high-contrast: active){& { + -ms-high-contrast-adjust: none; + } data-icon-name="StatusCircleCheckmark" role="presentation" /> diff --git a/scripts/tasks/webpack-resources.js b/scripts/tasks/webpack-resources.js index 91adf80278493f..6bc227592c55ed 100644 --- a/scripts/tasks/webpack-resources.js +++ b/scripts/tasks/webpack-resources.js @@ -74,6 +74,7 @@ module.exports = { { devServer: { inline: true, + host: '10.121.24.189', port: 4322, }, From 6e9c8a23a8c919b0e281b589a98233eaa8f1705b Mon Sep 17 00:00:00 2001 From: Lambert W Date: Fri, 23 Feb 2018 13:43:57 -0800 Subject: [PATCH 8/8] Fix typo --- .../src/components/ContextualMenu/ContextualMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx index 08122bb85bba16..a9a6de851776d2 100644 --- a/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx +++ b/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.tsx @@ -532,7 +532,7 @@ export class ContextualMenu extends BaseComponent