fix(side-drawer): refactor side drawer (#1022)

* Refactor to place constants closer to context * Refactor events tests to reflect their true actions * Test the show an hide methods * Change close to hide * Prettier * Remove console.log * Remove comments from non API functions * Delete openChanged method * Make event handlers private * Effectively remove the listener * Test keypress * Prettier * Remove the observer and the empty constructor * Added missing "addElement" * Refactor tests a bit * Oopsy submitting `only` in tests * Add onKeydown to scrim Clickable non-interactive elements must have at least 1 keyboard event listener lit-a11y/click-events-have-key-events Co-authored-by: rinaok <[email protected]>
Vonage · Aug 17, 2021 · 2788c2b · 2788c2b
1 parent 65cfe63
commit 2788c2b
Show file tree

Hide file tree

Showing 2 changed files with 182 additions and 186 deletions.
diff --git a/components/side-drawer/src/vwc-side-drawer-base.ts b/components/side-drawer/src/vwc-side-drawer-base.ts
@@ -6,7 +6,6 @@ import {
 } from 'lit-element';
 import { ifDefined } from 'lit-html/directives/if-defined';
 import { classMap } from 'lit-html/directives/class-map';
-import { observer } from '@material/mwc-base/observer';
 import { DocumentWithBlockingElements } from 'blocking-elements';
 
 const blockingElements =
@@ -18,64 +17,51 @@ export class VWCSideDrawerBase extends LitElement {
 	 * accepts boolean value
 	 * @public
 	 * */
-	@property({ type: Boolean, reflect: true })
+	@property({
+		type: Boolean,
+		reflect: true
+	})
 	alternate = false;
 
 	/**
 	 * @prop hasTopBar - adds top bar to the side drawer
 	 * accepts boolean value
 	 * @public
 	 * */
-	@property({ type: Boolean, reflect: true })
+	@property({
+		type: Boolean,
+		reflect: true
+	})
 	hasTopBar?: boolean;
 
 	/**
 	 * @prop type - can be modal, dismissible or empty
 	 * accepts String value
 	 * @public
 	 * */
-	@property({ type: String, reflect: true })
+	@property({
+		type: String,
+		reflect: true
+	})
 	type = '';
 
 	/**
 	 * @prop absolute - the modal can be fixed or absolute
 	 * accepts Boolean value
 	 * @public
 	 * */
-	@property({ type: Boolean, reflect: true })
+	@property({
+		type: Boolean,
+		reflect: true
+	})
 	absolute = false;
 
-	@property({ type: Boolean, reflect: true })
-	@observer(function (
-		this: VWCSideDrawerBase,
-		isOpen: boolean,
-		wasOpen: boolean
-	) {
-		if (isOpen) {
-			this.show();
-			// wasOpen helps with first render (when it is `undefined`) perf
-		} else if (wasOpen !== undefined) {
-			this.close();
-		}
-		this.openChanged(isOpen);
+	@property({
+		type: Boolean,
+		reflect: true
 	})
 	open = false;
-	/**
-	 * Invoked when the element open state is updated.
-	 *
-	 * Expressions inside this method will trigger upon open state change.
-	 *
-	 * @param _isOpen Boolean of open state
-	 */ openChanged(
-		// eslint-disable-next-line @typescript-eslint/no-unused-vars
-		_isOpen: boolean
-		// eslint-disable-next-line @typescript-eslint/no-empty-function
-	): void {}
 
-	constructor() {
-		super();
-		this.addEventListener('transitionend', () => this.onTransitionEnd());
-	}
 	/**
 	 * Opens the side drawer from the closed state.
 	 * @public
@@ -88,175 +74,131 @@ export class VWCSideDrawerBase extends LitElement {
 	 * Closes the side drawer from the open state.
 	 * @public
 	 */
-	close(): void {
+	hide(): void {
 		this.open = false;
 	}
 
-	/**
-	 * Side drawer finished open animation.
-	 */
-	#opened(): void {
-		this.#trapFocus();
-		this.#notifyOpen();
+	connectedCallback() {
+		super.connectedCallback();
+		this.addEventListener('transitionend', this.#onTransitionEnd);
+		this.addEventListener('keydown', this.#onKeydown);
 	}
 
-	/**
-	 * Side drawer finished close animation.
-	 */
-	#closed(): void {
+	disconnectedCallback(): void {
+		super.disconnectedCallback();
 		this.#releaseFocus();
-		this.#notifyClose();
+		this.removeEventListener('transitionend', this.#onTransitionEnd);
+		this.removeEventListener('keydown', this.#onKeydown);
 	}
 
-	/**
-	 * DispatchEvent creator.
-	 * @param eventName
-	 */
-	#createDispatchEvent(eventName: string) {
-		const init: CustomEventInit = { bubbles: true, composed: true };
-		const ev = new CustomEvent(eventName, init);
-		this.dispatchEvent(ev);
-	}
+	protected render(): TemplateResult {
+		const dismissible = this.type === 'dismissible' || this.type === 'modal';
+		const modal = this.type === 'modal';
+		const topBar = this.hasTopBar ? this.renderTopBar() : '';
+		const scrim = this.type === 'modal' && this.open ? this.renderScrim() : '';
+		const alternate = this.alternate ? 'vvd-scheme-alternate' : undefined;
+		const absolute = this.type === 'modal' && this.absolute;
 
-	/**
-	 * Notify close.
-	 *
-	 * @fires SideDrawer#closed
-	 */
-	#notifyClose(): void {
-		this.#createDispatchEvent('closed');
-	}
+		const classes = {
+			'vvd-side-drawer--alternate': this.alternate,
+			'vvd-side-drawer--dismissible': dismissible,
+			'vvd-side-drawer--modal': modal,
+			'vvd-side-drawer--open': this.open,
+			'vvd-side-drawer--absolute': absolute,
+		};
 
-	/**
-	 * Notify open.
-	 *
-	 * @fires SideDrawer#opened
-	 */
-	#notifyOpen(): void {
-		this.#createDispatchEvent('opened');
-	}
+		return html`
+			<aside
+				part="${ifDefined(alternate)}"
+				class="side-drawer ${classMap(classes)}"
+			>
+				${topBar}
 
-	disconnectedCallback(): void {
-		super.disconnectedCallback();
-		this.#releaseFocus();
-		this.removeEventListener('transitionend', () => this.onTransitionEnd());
+				<div class="vvd-side-drawer--content">
+					<slot></slot>
+				</div>
+			</aside>
+
+			${scrim}
+		`;
 	}
 
-	/**
-	 * Traps focus on root element and focuses the active navigation element.
-	 *
-	 * Notify trap focus.
-	 * @fires SideDrawer#trapFocus
-	 */
-	#trapFocus(): void {
-		blockingElements.push(this);
-		this.#createDispatchEvent('trapFocus');
+	private renderTopBar(): TemplateResult {
+		return html`
+			<div class="vvd-side-drawer--top-bar">
+				<slot name="top-bar"></slot>
+			</div>`;
 	}
 
-	/**
-	 * Releases focus trap from root element which was set by `trapFocus`.
-	 *
-	 * Notify release focus.
-	 * @fires SideDrawer#releaseFocus
-	 */
-	#releaseFocus(): void {
-		blockingElements.remove(this);
-		this.#createDispatchEvent('releaseFocus');
+	private renderScrim(): TemplateResult {
+		return html`
+			<div
+				class="vvd-side-drawer--scrim ${this.absolute
+		? 'vvd-side-drawer--absolute'
+		: ''}"
+				@click="${this.#handleScrimClick}"
+				@onKeydown="${this.#handleScrimClick}"
+			></div>`;
 	}
 
-	/**
-	 * Click handler to close side drawer when scrim is clicked.
-	 */
-	handleScrimClick(): void {
+	#handleScrimClick(): void {
 		if (this.type === 'modal' && this.open) {
-			this.close();
+			this.hide();
 		}
 	}
 
-	/**
-	 * Keydown handler to close side drawer when key is escape.
-	 */
-	onKeydown({ key }: KeyboardEvent): void {
-		console.log(this.type, this.open, key);
+	#onKeydown = ({ key }: KeyboardEvent): void => {
 		if (this.type === 'modal' && this.open && key === 'Escape') {
-			this.close();
+			this.hide();
 		}
-	}
+	};
 
-	/**
-	 * Handles the `transitionend` event when the side drawer finishes opening/closing.
-	 */
-	onTransitionEnd(): void {
+	#onTransitionEnd = (): void => {
 		if (this.type === 'modal') {
 			// when side drawer finishes open animation
 			if (this.open) {
 				this.#opened();
 			} else {
-				// when side drawer finishes close animation
+				// when side drawer finishes hide animation
 				this.#closed();
 			}
 		}
-	}
+	};
 
-	/**
-	 * renderTopBar
-	 * @slot top-bar
-	 * @returns TemplateResult
-	 */
-	private renderTopBar(): TemplateResult {
-		return html`<div class="vvd-side-drawer--top-bar">
-			<slot name="top-bar"></slot>
-		</div>`;
+	#opened(): void {
+		this.#trapFocus();
+		this.#notifyOpen();
 	}
 
-	/**
-	 * renderScrim
-	 * @returns TemplateResult
-	 */
-	private renderScrim(): TemplateResult {
-		// eslint-disable-next-line lit-a11y/click-events-have-key-events
-		return html`<div
-			class="vvd-side-drawer--scrim ${this.absolute
-		? 'vvd-side-drawer--absolute'
-		: ''}"
-			@click="${this.handleScrimClick}"
-		></div>`;
+	#closed(): void {
+		this.#releaseFocus();
+		this.#notifyClose();
 	}
 
-	/**
-	 * the html markup
-	 * @internal
-	 * */
-	protected render(): TemplateResult {
-		const dismissible = this.type === 'dismissible' || this.type === 'modal';
-		const modal = this.type === 'modal';
-		const topBar = this.hasTopBar ? this.renderTopBar() : '';
-		const scrim = this.type === 'modal' && this.open ? this.renderScrim() : '';
-		const alternate = this.alternate ? 'vvd-scheme-alternate' : undefined;
-		const absolute = this.type === 'modal' && this.absolute;
-
-		const classes = {
-			'vvd-side-drawer--alternate': this.alternate,
-			'vvd-side-drawer--dismissible': dismissible,
-			'vvd-side-drawer--modal': modal,
-			'vvd-side-drawer--open': this.open,
-			'vvd-side-drawer--absolute': absolute,
+	#createDispatchEvent(eventName: string) {
+		const init: CustomEventInit = {
+			bubbles: true,
+			composed: true
 		};
+		const ev = new CustomEvent(eventName, init);
+		this.dispatchEvent(ev);
+	}
 
-		return html`
-			<aside
-				part="${ifDefined(alternate)}"
-				class="side-drawer ${classMap(classes)}"
-				@keydown=${this.onKeydown}
-			>
-				${topBar}
+	#notifyClose(): void {
+		this.#createDispatchEvent('closed');
+	}
 
-				<div class="vvd-side-drawer--content">
-					<slot></slot>
-				</div>
-			</aside>
+	#notifyOpen(): void {
+		this.#createDispatchEvent('opened');
+	}
 
-			${scrim}
-		`;
+	#trapFocus(): void {
+		blockingElements.push(this);
+		this.#createDispatchEvent('trapFocus');
+	}
+
+	#releaseFocus(): void {
+		blockingElements.remove(this);
+		this.#createDispatchEvent('releaseFocus');
 	}
 }