Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
07b16f9
Add autoexpand on touch to align with the desired behavior
jspurlin Mar 12, 2018
3502f3a
update to use pointer events for comboBox and splitButton
jspurlin Mar 14, 2018
1deccdd
update snapshot
jspurlin Mar 14, 2018
4f36e14
remove unused variable
jspurlin Mar 14, 2018
06806a1
Add preventDefault and stopImmediatePropagation so that IE/Edge will …
jspurlin Mar 14, 2018
e6e0276
test change
Mar 22, 2018
78ee211
removed test push
Mar 22, 2018
aea2641
resolved merge conflict
Mar 23, 2018
020ed81
added change file
Mar 23, 2018
c60caca
update bundle size
Mar 23, 2018
8b888d8
temp combo box changes
Mar 27, 2018
fbb8857
added touch start event and added test cases
Mar 30, 2018
4c5720e
updated tests with else case
Apr 3, 2018
90c6ac1
resolved merge
Apr 3, 2018
b8b07c9
removed unneeded comment
Apr 3, 2018
fab1029
updated change list
Apr 3, 2018
0293d75
updated code to deal with async race conditions
Apr 3, 2018
fa7671e
removed stopping evnet handling that stops firefox from opening the g…
Apr 4, 2018
c284b76
cleaned up the code; made sure to cancel actions when they're finished
Apr 4, 2018
d6be2f4
fixed minor nitpick on style
Apr 4, 2018
30a2933
added if case to deal with touch event if there is no onclick; remove…
Apr 4, 2018
1fe7db7
removed unnecessary imports to reduce file size
Apr 5, 2018
ad563fe
fixed build problems; added constants
Apr 5, 2018
1702de4
changed readonly to const
Apr 5, 2018
f20265a
solved build problem with string types
Apr 5, 2018
ec0bd53
fixed missing const that's causing build break with defined type
Apr 5, 2018
51e730f
resolved merge
Apr 9, 2018
f8fea3b
resolved merge; moved reference of splitbutton container to component…
chang47 Apr 11, 2018
dd096d8
Merge https://github.com/OfficeDev/office-ui-fabric-react into jspurl…
chang47 Apr 11, 2018
acce342
added comment for time out
chang47 Apr 11, 2018
dbaa322
increased max size limit of bundle
chang47 Apr 11, 2018
e3ebdf7
Merge branch 'master' into jspurlin/ComboBoxExpandOnTouch
dzearing Apr 12, 2018
e57e6e9
resolve merge conflict
Apr 12, 2018
dd13382
updated contextual menu to address comments
Apr 12, 2018
089233c
added issue regarding dynamically generating a primary action into th…
Apr 12, 2018
a94b7c5
increased the size limit
Apr 12, 2018
71ade9a
updated with better comments and grammatical fixes
Apr 12, 2018
b68bf62
pulled latest from fabric
chang47 Apr 30, 2018
16a88a7
fixed edge async menu close bug
chang47 Apr 30, 2018
cec47d0
update test
chang47 Apr 30, 2018
6d3810e
bumped bundle size
chang47 Apr 30, 2018
1f9fd86
fixed gammar and moved some code around
chang47 Apr 30, 2018
8ce9bc7
made better callback name
chang47 Apr 30, 2018
9b45fef
added pointer and touch event
chang47 Apr 30, 2018
fb63fb8
fixed semicolon
chang47 Apr 30, 2018
e7d0229
updated bundle size
May 1, 2018
9d247be
resovled merge
May 2, 2018
d3b1d13
fixed bundle size to include keytip change
May 2, 2018
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "SplitButton/ComboBox: added onTouch support for menu expansion.",
"type": "minor"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ContextualMenu, IContextualMenuProps } from '../../ContextualMenu';
import { IButtonProps, IButton } from './Button.types';
import { IButtonClassNames, getBaseButtonClassNames } from './BaseButton.classNames';
import { getClassNames as getBaseSplitButtonClassNames, ISplitButtonClassNames } from './SplitButton/SplitButton.classNames';
import { TouchEvent } from 'react';

export interface IBaseButtonProps extends IButtonProps {
baseClassName?: string;
Expand Down Expand Up @@ -49,6 +50,8 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
private _descriptionId: string;
private _ariaDescriptionId: string;
private _classNames: IButtonClassNames;
private _processingTouch: boolean;
private _lastTouchTimeoutId: number | undefined;

constructor(props: IBaseButtonProps, rootClassName: string) {
super(props);
Expand Down Expand Up @@ -192,6 +195,12 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
return this._onRenderContent(tag, buttonProps);
}

public componentDidMount() {
if (this._isSplitButton && this._splitButtonContainer.value && 'onpointerdown' in this._splitButtonContainer.value) {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps leave a comment here on why! That way the person re-writing the button later can understand your motivation.

// For split buttons, touching anywhere in the button should drop the dropdown, which should contain the primary action. This gives more hit target space for touch environments.

this._events.on(this._splitButtonContainer.value, 'pointerdown', this._onPointerDown, true);
Copy link
Member

Choose a reason for hiding this comment

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

can't you use react eventing? You should avoid using native events when possible.

Copy link
Contributor

@joschect joschect Mar 26, 2018

Choose a reason for hiding this comment

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

React does not support this yet, you could potentially mix it in with a ... but you won't get type safety. Based on this issue facebook/react#499 it looks like there was some disagreement within the react community as to whether or not pointer events should get added since they still aren't supported by IOS but someone could probably add that now.

}
}

public componentDidUpdate(prevProps: IBaseButtonProps, prevState: IBaseButtonState) {
// If Button's menu was closed, run onAfterMenuDismiss
if (this.props.onAfterMenuDismiss && prevState.menuProps && !this.state.menuProps) {
Expand Down Expand Up @@ -425,7 +434,6 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
disabled,
checked,
getSplitButtonClassNames,
onClick,
primaryDisabled
} = this.props;

Expand Down Expand Up @@ -458,6 +466,7 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
aria-describedby={ buttonProps.ariaDescription }
className={ classNames && classNames.splitButtonContainer }
onKeyDown={ this._onSplitButtonContainerKeyDown }
onTouchStart={ this._onTouchStart }
ref={ this._splitButtonContainer }
data-is-focusable={ true }
onClick={ !disabled && !primaryDisabled ? this._onSplitButtonPrimaryClick : undefined }
Expand All @@ -478,8 +487,11 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
if (this._isExpanded) {
this._dismissMenu();
}
if (this.props.onClick) {

if (!this._processingTouch && this.props.onClick) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now if we are not processingTouch but there also isn't a this.porps.onClick we will call this._onMenuClick where as before we would not call this.onMenuClick, is that expected?

Copy link
Contributor Author

@chang47 chang47 Apr 4, 2018

Choose a reason for hiding this comment

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

You're correct, I've changed it to an else if to only do a menu click if we're processingTouch

this.props.onClick(ev);
} else {
this._onMenuClick(ev);
}
}

Expand Down Expand Up @@ -520,6 +532,14 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
return <BaseButton {...splitButtonProps} onMouseDown={ this._onMouseDown } tabIndex={ -1 } />;
}

private _onSplitButtonClick = (ev: React.MouseEvent<HTMLDivElement | HTMLAnchorElement>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method isn't used anywhere, how is it getting called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it's not this was old code that came from a merge conflict. I'll get rid of it.

if (!this._processingTouch && this.props.onClick) {
this.props.onClick(ev);
} else {
this._onMenuClick(ev);
}
}

private _onMouseDown = (ev: React.MouseEvent<BaseButton>) => {
if (this.props.onMouseDown) {
this.props.onMouseDown(ev);
Expand Down Expand Up @@ -563,6 +583,36 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
}
}

private _onTouchStart: (ev: TouchEvent<HTMLElement>) => void = (ev) => {
if (this._isSplitButton && this._splitButtonContainer.value && !('onpointerdown' in this._splitButtonContainer.value)) {
this._handleTouchAndPointerEvent();
}
}

private _onPointerDown(ev: PointerEvent) {
if (ev.pointerType === 'touch') {
this._handleTouchAndPointerEvent();

ev.preventDefault();
ev.stopImmediatePropagation();
}
}

private _handleTouchAndPointerEvent() {
// If we already have an existing timeeout from a previous touch and pointer event
// cancel that timeout so we can set a nwe one.
if (this._lastTouchTimeoutId !== undefined) {
this._async.clearTimeout(this._lastTouchTimeoutId);
this._lastTouchTimeoutId = undefined;
}
this._processingTouch = true;

this._lastTouchTimeoutId = this._async.setTimeout(() => {
this._processingTouch = false;
this._lastTouchTimeoutId = undefined;
}, 500);
}

/**
* Returns if the user hits a valid keyboard key to open the menu
* @param ev - the keyboard event
Expand All @@ -576,7 +626,7 @@ export class BaseButton extends BaseComponent<IBaseButtonProps, IBaseButtonState
}
}

private _onMenuClick = (ev: React.MouseEvent<HTMLAnchorElement>) => {
private _onMenuClick = (ev: React.MouseEvent<HTMLDivElement | HTMLAnchorElement>) => {
const { onMenuClick } = this.props;
if (onMenuClick) {
onMenuClick(ev, this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,40 @@ describe('Button', () => {
expect(renderedDOM.getAttribute('aria-expanded')).toEqual('true');
});

it('Touch Start on primary button of SplitButton expands menu', () => {
const button = ReactTestUtils.renderIntoDocument<any>(
<DefaultButton
data-automation-id='test'
text='Create account'
split={ true }
onClick={ alertClicked }
menuProps={ {
items: [
{
key: 'emailMessage',
name: 'Email message',
icon: 'Mail'
},
{
key: 'calendarEvent',
name: 'Calendar event',
icon: 'Calendar'
}
]
} }
/>
);
const renderedDOM = ReactDOM.findDOMNode(button as React.ReactInstance);
const primaryButtonDOM: HTMLButtonElement = renderedDOM.getElementsByTagName('button')[0] as HTMLButtonElement;

// in a normal scenario, when we do a touchstart we would also cause a
// click event to fire. This doesn't happen in the simulator so we're
// manually adding this in.
ReactTestUtils.Simulate.touchStart(primaryButtonDOM);
ReactTestUtils.Simulate.click(primaryButtonDOM);
expect(renderedDOM.getAttribute('aria-expanded')).toEqual('true');
});

it('If menu trigger is disabled, pressing down does not trigger menu', () => {
const button = ReactTestUtils.renderIntoDocument<any>(
<DefaultButton
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,32 @@ describe('ComboBox', () => {
expect(returnUndefined.mock.calls.length).toBe(1);
});

it('Call onMenuOpened when touch start on the input', () => {
let comboBoxRoot;
let inputElement;
const returnUndefined = jest.fn();

const wrapper = mount(
<ComboBox
label='testgroup'
defaultSelectedKey='1'
options={ DEFAULT_OPTIONS2 }
onMenuOpen={ returnUndefined }
allowFreeform={ true }
/>);
comboBoxRoot = wrapper.find('.ms-ComboBox');

inputElement = comboBoxRoot.find('input');

// in a normal scenario, when we do a touchstart we would also cause a
// click event to fire. This doesn't happen in the simulator so we're
// manually adding this in.
inputElement.simulate('touchstart');
inputElement.simulate('click');

expect(wrapper.find('.is-open').length).toEqual(1);
});

it('Can type a complete option with autocomplete and allowFreeform on and submit it', () => {
let updatedOption;
let updatedIndex;
Expand Down
Loading