Person-card fixed touch in person component + login refactor (window event) (#228)

* [mgt-person] adding touch event to window to help with hiding person-card

* [mgt-login] moving click handle to connected and disconnected callback methods

* [mgt-person] refactor touch and click events

* [mgt-person] re-adding logic for showpersoncard onclick

* [person] separating openPersonCard click to not be attached to window unfactor :(

* [mgt-person] fix for rapid open close of person card in touch events

* making private variable instead of property, touch disables handleMouseEnter timeouts

* removed touch handling and only depend on click event for everything

* Clarified window click handler names

Author: vogtn

src/components/mgt-login/mgt-login.ts

@@ -62,6 +62,27 @@ export class MgtLogin extends MgtBaseComponent {
     super();
     Providers.onProviderUpdated(() => this.loadState());
     this.loadState();
+    this.handleWindowClick = this.handleWindowClick.bind(this);
+  }
+
+  /**
+   * Invoked each time the custom element is appended into a document-connected element
+   *
+   * @memberof MgtLogin
+   */
+  public connectedCallback() {
+    super.connectedCallback();
+    window.addEventListener('click', this.handleWindowClick);
+  }
+
+  /**
+   * Invoked each time the custom element is disconnected from the document's DOM
+   *
+   * @memberof MgtLogin
+   */
+  public disconnectedCallback() {
+    window.removeEventListener('click', this.handleWindowClick);
+    super.disconnectedCallback();
   }
 
   /**
@@ -159,26 +180,6 @@ export class MgtLogin extends MgtBaseComponent {
     }
   }
 
-  /**
-   * Invoked when the element is first updated. Implement to perform one time
-   * work on the element after update.
-   *
-   * Setting properties inside this method will trigger the element to update
-   * again after this update cycle completes.
-   *
-   * * @param _changedProperties Map of changed properties with old values
-   */
-  protected firstUpdated() {
-    window.addEventListener('click', (event: MouseEvent) => {
-      // get popup bounds
-      const popup = this.renderRoot.querySelector('.popup');
-      if (popup) {
-        this._popupRect = popup.getBoundingClientRect();
-        this._showMenu = false;
-      }
-    });
-  }
-
   /**
    * Invoked on each update to perform rendering tasks. This method must return
    * a lit-html TemplateResult. Setting properties inside this method will *not*
@@ -197,6 +198,19 @@ export class MgtLogin extends MgtBaseComponent {
     `;
   }
 
+  private handleWindowClick(e: MouseEvent) {
+    if (e.target === this) {
+      return;
+    }
+
+    // get popup bounds
+    const popup = this.renderRoot.querySelector('.popup');
+    if (popup) {
+      this._popupRect = popup.getBoundingClientRect();
+      this._showMenu = false;
+    }
+  }
+
   private renderLogIn() {
     return html`
       <i class="login-icon ms-Icon ms-Icon--Contact"></i>
@@ -270,7 +284,6 @@ export class MgtLogin extends MgtBaseComponent {
   }
 
   private onClick(event: MouseEvent) {
-    event.stopPropagation();
     if (this.userDetails) {
       // get login button bounds
       const loginButton = this.renderRoot.querySelector('.login-button');

src/components/mgt-person/mgt-person.scss

@@ -20,7 +20,7 @@ $email-font-size: var(--email-font-size, #{$ms-font-size-s});
 $email-color: var(--email-color, #{$ms-color-neutralPrimary});
 
 :host {
-  display: inherit;
+  display: inline-block;
 }
 
 :host .root {

src/components/mgt-person/mgt-person.ts

@@ -8,7 +8,6 @@
 import * as MicrosoftGraph from '@microsoft/microsoft-graph-types';
 import { customElement, html, property, PropertyValues, TemplateResult } from 'lit-element';
 import { classMap } from 'lit-html/directives/class-map';
-import { styleMap } from 'lit-html/directives/style-map';
 import { Providers } from '../../Providers';
 import { ProviderState } from '../../providers/IProvider';
 import '../../styles/fabric-icon-font';
@@ -124,6 +123,11 @@ export class MgtPerson extends MgtTemplatedComponent {
   private _mouseLeaveTimeout;
   private _mouseEnterTimeout;
 
+  constructor() {
+    super();
+    this.handleWindowClick = this.handleWindowClick.bind(this);
+  }
+
   /**
    * Synchronizes property values when attributes change.
    *
@@ -156,6 +160,26 @@ export class MgtPerson extends MgtTemplatedComponent {
     this.loadData();
   }
 
+  /**
+   * Invoked each time the custom element is appended into a document-connected element
+   *
+   * @memberof MgtPerson
+   */
+  public connectedCallback() {
+    super.connectedCallback();
+    window.addEventListener('click', this.handleWindowClick);
+  }
+
+  /**
+   * Invoked each time the custom element is disconnected from the document's DOM
+   *
+   * @memberof MgtPerson
+   */
+  public disconnectedCallback() {
+    window.removeEventListener('click', this.handleWindowClick);
+    super.disconnectedCallback();
+  }
+
   /**
    * Invoked on each update to perform rendering tasks. This method must return
    * a lit-html TemplateResult. Setting properties inside this method will *not*
@@ -174,9 +198,9 @@ export class MgtPerson extends MgtTemplatedComponent {
     return html`
       <div
         class="root"
+        @click=${this.handleMouseClick}
         @mouseenter=${this.handleMouseEnter}
         @mouseleave=${this.handleMouseLeave}
-        @click=${this.handleMouseClick}
       >
         ${this.renderFlyout(person)}
       </div>
@@ -203,6 +227,12 @@ export class MgtPerson extends MgtTemplatedComponent {
     }
   }
 
+  private handleWindowClick(e: MouseEvent) {
+    if (this.isPersonCardVisible && e.target !== this) {
+      this.hidePersonCard();
+    }
+  }
+
   private async loadData() {
     const provider = Providers.globalProvider;
 
@@ -288,11 +318,9 @@ export class MgtPerson extends MgtTemplatedComponent {
     this.requestUpdate();
   }
 
-  private handleMouseClick() {
-    if (this.personCardInteraction === PersonCardInteraction.click && !this.isPersonCardVisible) {
+  private handleMouseClick(e: MouseEvent) {
+    if (this.personCardInteraction !== PersonCardInteraction.none && !this.isPersonCardVisible) {
       this.showPersonCard();
-    } else {
-      this.hidePersonCard();
     }
   }