remove customized dismiss function in hover card (microsoft#3489)

* remove customized dismiss function in hover card * rush change * add ref in tile component * rush change in experiments * change comments * change hot key for hover card to "c"
chrismohr · Apr 17, 2018 · b35d7b1 · b35d7b1
1 parent 0ca27f0
commit b35d7b1
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 19 deletions.
diff --git a/.../changes/@uifabric/experiments/chf-2017-11-hoverCard_EventHandleFix_2017-12-01-00-46.json b/.../changes/@uifabric/experiments/chf-2017-11-hoverCard_EventHandleFix_2017-12-01-00-46.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@uifabric/experiments",
+      "comment": "add link ref in tile",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@uifabric/experiments",
+  "email": "chf@microsoft.com"
+}
diff --git a/...changes/office-ui-fabric-react/chf-2017-11-hoverCard_EventHandleFix_2017-11-30-19-26.json b/...changes/office-ui-fabric-react/chf-2017-11-hoverCard_EventHandleFix_2017-11-30-19-26.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "remove customized dismiss function in HoverCard",
+      "type": "patch"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "chf@microsoft.com"
+}
diff --git a/packages/experiments/src/components/Tile/Tile.tsx b/packages/experiments/src/components/Tile/Tile.tsx
@@ -204,6 +204,7 @@ export class Tile extends BaseComponent<ITileProps, ITileState> {
         <a
           href={ href }
           onClick={ onClick }
+          ref={ this.props.linkRef }
           data-selection-invoke={ (isInvokable && selectionIndex > -1) ? true : undefined }
           className={ css('ms-Tile-link', TileStyles.link) }
         >

diff --git a/packages/experiments/src/components/Tile/Tile.types.ts b/packages/experiments/src/components/Tile/Tile.types.ts
@@ -124,4 +124,11 @@ export interface ITileProps extends IBaseProps, React.AllHTMLAttributes<HTMLSpan
    * @memberof ITileProps
    */
   toggleSelectionAriaLabel?: boolean;
+
+  /**
+   * Link ref
+   * @type {() => void}
+   * @memberof ITileProps
+   */
+  linkRef?: (element: HTMLAnchorElement) => void;
 }
diff --git a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx
@@ -39,10 +39,10 @@ export class FocusTrapZone extends BaseComponent<IFocusTrapZoneProps, {}> implem
   }
 
   public componentDidMount() {
-    let { isClickableOutsideFocusTrap = false, forceFocusInsideTrap = true, elementToFocusOnDismiss } = this.props;
+    let { isClickableOutsideFocusTrap = false, forceFocusInsideTrap = true, elementToFocusOnDismiss, disableFirstFocus = false } = this.props;
 
     this._previouslyFocusedElement = elementToFocusOnDismiss ? elementToFocusOnDismiss : document.activeElement as HTMLElement;
-    if (!elementContains(this.refs.root, this._previouslyFocusedElement)) {
+    if (!elementContains(this.refs.root, this._previouslyFocusedElement) && !disableFirstFocus) {
       this.focus();
     }
 
@@ -94,6 +94,8 @@ export class FocusTrapZone extends BaseComponent<IFocusTrapZoneProps, {}> implem
         ref='root'
         aria-labelledby={ ariaLabelledBy }
         onKeyDown={ this._onKeyboardHandler }
+        tabIndex={ -1 }
+        onFocus={ this._onRootFocus }
       >
         { this.props.children }
       </div>
@@ -122,6 +124,13 @@ export class FocusTrapZone extends BaseComponent<IFocusTrapZoneProps, {}> implem
     }
   }
 
+  @autobind
+  private _onRootFocus(ev: React.FocusEvent<HTMLElement>) {
+    if (ev.target === this.refs.root) {
+      this.focus();
+    }
+  }
+
   @autobind
   private _onKeyboardHandler(ev: React.KeyboardEvent<HTMLElement>) {
     if (ev.which !== KeyCodes.tab) {

diff --git a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.types.ts b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.types.ts
@@ -47,4 +47,10 @@ export interface IFocusTrapZoneProps extends React.HTMLAttributes<HTMLDivElement
   * Indicates the selector for first focusable item
   */
   firstFocusableSelector?: string | (() => string);
+
+  /**
+   * Do not put focus onto first element when render focus trap zone
+   * @default false
+   */
+  disableFirstFocus?: boolean;
 }
diff --git a/packages/office-ui-fabric-react/src/components/HoverCard/ExpandingCard.tsx b/packages/office-ui-fabric-react/src/components/HoverCard/ExpandingCard.tsx
@@ -6,6 +6,7 @@ import {
   getNativeProps,
   divProperties,
   customizable,
+  KeyCodes,
   autobind
 } from '../../Utilities';
 import { IExpandingCardProps, IExpandingCardStyles, ExpandingCardMode } from './ExpandingCard.types';
@@ -54,7 +55,6 @@ export class ExpandingCard extends BaseComponent<IExpandingCardProps, IExpanding
     }
 
     if (this.props.trapFocus) {
-      this._expandingCard.focus();
       this._events.on(this._expandingCard, 'keydown', this._onDismiss);
     }
   }
@@ -70,15 +70,14 @@ export class ExpandingCard extends BaseComponent<IExpandingCardProps, IExpanding
       styles: customStyles,
       compactCardHeight,
       directionalHintFixed,
+      firstFocus,
       expandedCardHeight
     } = this.props;
     this._styles = getStyles(theme!, customStyles);
 
     const content = (
       <div
         ref={ this._resolveRef('_expandingCard') }
-        onFocusCapture={ this.props.onEnter }
-        onBlurCapture={ this.props.onLeave }
         onMouseEnter={ this.props.onEnter }
         onMouseLeave={ this.props.onLeave }
       >
@@ -101,10 +100,11 @@ export class ExpandingCard extends BaseComponent<IExpandingCardProps, IExpanding
         directionalHintFixed={ directionalHintFixed }
         finalHeight={ compactCardHeight! + expandedCardHeight! }
         minPagePadding={ 24 }
+        onDismiss={ this.props.onLeave }
         gapSpace={ this.props.gapSpace }
       >
         { this.props.trapFocus ?
-          <FocusTrapZone>
+          <FocusTrapZone forceFocusInsideTrap={ false } isClickableOutsideFocusTrap={ true } disableFirstFocus={ !firstFocus }>
             { content }
           </FocusTrapZone> :
           content
@@ -115,7 +115,9 @@ export class ExpandingCard extends BaseComponent<IExpandingCardProps, IExpanding
 
   @autobind
   private _onDismiss(ev: MouseEvent): void {
-    if (ev.type === 'keydown') {
+    if (ev.type === 'keydown' && (ev.which !== KeyCodes.escape)) {
+      return;
+    } else {
       this.props.onLeave && this.props.onLeave(ev);
     }
   }

diff --git a/packages/office-ui-fabric-react/src/components/HoverCard/ExpandingCard.types.ts b/packages/office-ui-fabric-react/src/components/HoverCard/ExpandingCard.types.ts
@@ -98,6 +98,11 @@ export interface IExpandingCardProps extends React.HTMLAttributes<HTMLDivElement
    * Trap focus or not
    */
   trapFocus?: boolean;
+
+  /**
+   * Focus on first element by default on card or not
+   */
+  firstFocus?: boolean;
 }
 
 export enum ExpandingCardMode {
@@ -112,6 +117,18 @@ export enum ExpandingCardMode {
   expanded = 1
 }
 
+export enum OpenCardMode {
+  /**
+   * Open card by hover
+   */
+  hover = 0,
+
+  /**
+   * Open card by hot key
+   */
+  hotKey = 1
+}
+
 export interface IExpandingCardStyles {
   /**
    * Style for the root element in the default enabled, non-toggled state.

diff --git a/packages/office-ui-fabric-react/src/components/HoverCard/HoverCard.tsx b/packages/office-ui-fabric-react/src/components/HoverCard/HoverCard.tsx
@@ -16,13 +16,14 @@ import {
 
 import { IHoverCardProps, IHoverCardStyles } from './HoverCard.types';
 import { ExpandingCard } from './ExpandingCard';
-import { ExpandingCardMode } from './ExpandingCard.types';
+import { ExpandingCardMode, OpenCardMode } from './ExpandingCard.types';
 import { getStyles } from './HoverCard.styles';
 import { FocusTrapZone } from 'office-ui-fabric-react/lib/FocusTrapZone';
 
 export interface IHoverCardState {
   isHoverCardVisible: boolean;
   mode: ExpandingCardMode;
+  openMode: OpenCardMode;
 }
 
 export class HoverCard extends BaseComponent<IHoverCardProps, IHoverCardState> {
@@ -49,7 +50,8 @@ export class HoverCard extends BaseComponent<IHoverCardProps, IHoverCardState> {
 
     this.state = {
       isHoverCardVisible: false,
-      mode: ExpandingCardMode.compact
+      mode: ExpandingCardMode.compact,
+      openMode: OpenCardMode.hover
     };
   }
 
@@ -99,7 +101,7 @@ export class HoverCard extends BaseComponent<IHoverCardProps, IHoverCardState> {
       setAriaDescribedBy = true,
       styles: customStyles
     } = this.props;
-    const { isHoverCardVisible, mode } = this.state;
+    const { isHoverCardVisible, mode, openMode } = this.state;
     const hoverCardId = id || getId('hoverCard');
 
     this._styles = getStyles(customStyles);
@@ -117,9 +119,10 @@ export class HoverCard extends BaseComponent<IHoverCardProps, IHoverCardState> {
             { ...getNativeProps(this.props, divProperties) }
             id={ hoverCardId }
             trapFocus={ !!this.props.trapFocus }
+            firstFocus={ openMode === OpenCardMode.hotKey }
             targetElement={ this._getTargetElement() }
             onEnter={ this._cardOpen }
-            onLeave={ this._cardDismiss }
+            onLeave={ this._executeCardDimiss }
             mode={ mode }
             { ...expandingCardProps }
           />
@@ -146,19 +149,25 @@ export class HoverCard extends BaseComponent<IHoverCardProps, IHoverCardState> {
   // Show HoverCard
   @autobind
   private _cardOpen(ev: MouseEvent) {
-    if (ev.type === 'keydown' && !(ev.shiftKey && ev.which === KeyCodes.space)) {
+    if (ev.type === 'keydown' && !(ev.which === KeyCodes.c)) {
       return;
     }
     this._async.clearTimeout(this._dismissTimerId);
     if (ev.type === 'mouseenter') {
       this._currentMouseTarget = ev.currentTarget;
     }
 
+    this._executeCardOpen(ev);
+  }
+
+  @autobind
+  private _executeCardOpen(ev: MouseEvent) {
     this._openTimerId = this._async.setTimeout(() => {
       if (!this.state.isHoverCardVisible) {
         this.setState({
           isHoverCardVisible: true,
-          mode: ExpandingCardMode.compact
+          mode: ExpandingCardMode.compact,
+          openMode: ev.type === 'keydown' ? OpenCardMode.hotKey : OpenCardMode.hover
         });
       }
     }, this.props.cardOpenDelay!);
@@ -174,15 +183,21 @@ export class HoverCard extends BaseComponent<IHoverCardProps, IHoverCardState> {
 
     // Dismiss if not sticky and currentTarget is the same element that mouse last entered
     if (!this.props.sticky && (this._currentMouseTarget === ev.currentTarget || (ev.which === KeyCodes.escape))) {
-      this._dismissTimerId = this._async.setTimeout(() => {
-        this.setState({
-          isHoverCardVisible: false,
-          mode: ExpandingCardMode.compact
-        });
-      }, this.props.cardDismissDelay!);
+      this._executeCardDimiss();
     }
   }
 
+  @autobind
+  private _executeCardDimiss() {
+    this._dismissTimerId = this._async.setTimeout(() => {
+      this.setState({
+        isHoverCardVisible: false,
+        mode: ExpandingCardMode.compact,
+        openMode: OpenCardMode.hover
+      });
+    }, this.props.cardDismissDelay!);
+  }
+
   // Instant Open the card in Expanded mode
   @autobind
   private _instantOpenExpanded(ev: React.MouseEvent<HTMLDivElement>) {

diff --git a/packages/utilities/src/KeyCodes.ts b/packages/utilities/src/KeyCodes.ts
@@ -5,6 +5,7 @@
  */
 export const enum KeyCodes {
   a = 65,
+  c = 67,
   backspace = 8,
   comma = 188,
   del = 46,