From 72aafdc8f4831117540b81bbde02bba77ecb2e45 Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Wed, 26 Jun 2024 10:52:03 +0200 Subject: [PATCH 1/7] test(sbb-clock): stabilize visual spec --- src/elements/clock/clock.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/elements/clock/clock.ts b/src/elements/clock/clock.ts index 957102055d..8bad779032 100644 --- a/src/elements/clock/clock.ts +++ b/src/elements/clock/clock.ts @@ -233,16 +233,10 @@ export class SbbClockElement extends LitElement { /** Stops the clock by removing all the animations. */ private async _stopClock(): Promise { clearInterval(this._handMovement); + console.log('stop', this._handMovement); if (this.now) { this._setHandsStartingPosition(); - - // Wait a tick to before animation is added. Otherwise, the animation gets not completely - // removed which can lead to a mispositioned seconds hand. - await new Promise((resolve) => setTimeout(resolve)); - - this._clockHandSeconds?.classList.add('sbb-clock__hand-seconds--initial-minute'); - this._clockHandHours?.classList.add('sbb-clock__hand-hours--initial-hour'); } else { this._removeSecondsAnimationStyles(); this._removeHoursAnimationStyles(); @@ -269,6 +263,7 @@ export class SbbClockElement extends LitElement { ADD_EVENT_LISTENER_OPTIONS, ); + console.log('start'); await new Promise(() => setTimeout(() => this._setHandsStartingPosition(), INITIAL_TIMEOUT_DURATION), ); From 98551c726a62770c1fd0ec75f13ab56c0237a695 Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Wed, 26 Jun 2024 10:56:16 +0200 Subject: [PATCH 2/7] chore(sbb-clock): remove logs --- src/elements/clock/clock.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/elements/clock/clock.ts b/src/elements/clock/clock.ts index 8bad779032..012e437e66 100644 --- a/src/elements/clock/clock.ts +++ b/src/elements/clock/clock.ts @@ -233,7 +233,6 @@ export class SbbClockElement extends LitElement { /** Stops the clock by removing all the animations. */ private async _stopClock(): Promise { clearInterval(this._handMovement); - console.log('stop', this._handMovement); if (this.now) { this._setHandsStartingPosition(); @@ -263,7 +262,6 @@ export class SbbClockElement extends LitElement { ADD_EVENT_LISTENER_OPTIONS, ); - console.log('start'); await new Promise(() => setTimeout(() => this._setHandsStartingPosition(), INITIAL_TIMEOUT_DURATION), ); From e1a28b4fe7c33fe45d2dd05bde9b689a3a2be9c6 Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Wed, 26 Jun 2024 11:29:31 +0200 Subject: [PATCH 3/7] chore(sbb-clock): update snapshots --- src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js b/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js index 259b9de222..6c3a45018e 100644 --- a/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js +++ b/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js @@ -35,7 +35,7 @@ snapshots["sbb-clock renders with fixed time DOM"] = ` `; From 27c5794560d18d8c128859c9b9242ca0583673db Mon Sep 17 00:00:00 2001 From: Tommmaso Menga Date: Wed, 26 Jun 2024 13:03:10 +0200 Subject: [PATCH 4/7] chore: empty commit From e8c5ed9db23ba0a23455557d16fd27a70d53979e Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Wed, 26 Jun 2024 15:54:27 +0200 Subject: [PATCH 5/7] fix: fix clock --- .../__snapshots__/clock.snapshot.spec.snap.js | 2 +- src/elements/clock/clock.ts | 25 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js b/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js index 6c3a45018e..501cc93cf9 100644 --- a/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js +++ b/src/elements/clock/__snapshots__/clock.snapshot.spec.snap.js @@ -35,7 +35,7 @@ snapshots["sbb-clock renders with fixed time DOM"] = ` `; diff --git a/src/elements/clock/clock.ts b/src/elements/clock/clock.ts index 012e437e66..95c8ff2b5f 100644 --- a/src/elements/clock/clock.ts +++ b/src/elements/clock/clock.ts @@ -185,7 +185,6 @@ export class SbbClockElement extends LitElement { this._clockHandSeconds?.classList.add('sbb-clock__hand-seconds--initial-minute'); this._clockHandHours?.classList.add('sbb-clock__hand-hours--initial-hour'); - this.style?.setProperty('--sbb-clock-animation-play-state', 'running'); this.toggleAttribute('data-initialized', true); } @@ -234,12 +233,8 @@ export class SbbClockElement extends LitElement { private async _stopClock(): Promise { clearInterval(this._handMovement); - if (this.now) { - this._setHandsStartingPosition(); - } else { - this._removeSecondsAnimationStyles(); - this._removeHoursAnimationStyles(); - } + this._removeSecondsAnimationStyles(); + this._removeHoursAnimationStyles(); this._clockHandHours?.removeEventListener('animationend', this._moveHoursHandFn); this._clockHandSeconds?.removeEventListener('animationend', this._moveMinutesHandFn); @@ -247,6 +242,16 @@ export class SbbClockElement extends LitElement { this._clockHandMinutes?.classList.add('sbb-clock__hand-minutes--no-transition'); this.style?.setProperty('--sbb-clock-animation-play-state', 'paused'); + + if (this.now) { + if (this._clockHandSeconds) { + this._clockHandSeconds.style.animation = ''; + // Trigger reflow + this._clockHandSeconds.offsetHeight; + this._clockHandSeconds.style.removeProperty('animation'); + } + this._setHandsStartingPosition(); + } } /** Starts the clock by defining the hands starting position then starting the animations. */ @@ -263,7 +268,11 @@ export class SbbClockElement extends LitElement { ); await new Promise(() => - setTimeout(() => this._setHandsStartingPosition(), INITIAL_TIMEOUT_DURATION), + setTimeout(() => { + this._setHandsStartingPosition(); + + this.style?.setProperty('--sbb-clock-animation-play-state', 'running'); + }, INITIAL_TIMEOUT_DURATION), ); } From f2ab70ef9f0382eff7db5a838c3234766f1c5e0f Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Wed, 26 Jun 2024 15:59:48 +0200 Subject: [PATCH 6/7] docs: better explanation --- src/elements/clock/clock.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/elements/clock/clock.ts b/src/elements/clock/clock.ts index 95c8ff2b5f..1b15732fc5 100644 --- a/src/elements/clock/clock.ts +++ b/src/elements/clock/clock.ts @@ -244,16 +244,26 @@ export class SbbClockElement extends LitElement { this.style?.setProperty('--sbb-clock-animation-play-state', 'paused'); if (this.now) { - if (this._clockHandSeconds) { - this._clockHandSeconds.style.animation = ''; - // Trigger reflow - this._clockHandSeconds.offsetHeight; - this._clockHandSeconds.style.removeProperty('animation'); - } + this._resetSecondsHandAnimation(); this._setHandsStartingPosition(); } } + /** + * Removing animation by overriding with empty string, + * then triggering a reflow and re add original animation by removing override. + * @private + */ + private _resetSecondsHandAnimation(): void { + if (!this._clockHandSeconds) { + return; + } + this._clockHandSeconds.style.animation = ''; + // Hack to trigger reflow + this._clockHandSeconds.offsetHeight; + this._clockHandSeconds.style.removeProperty('animation'); + } + /** Starts the clock by defining the hands starting position then starting the animations. */ private async _startClock(): Promise { this._clockHandHours?.addEventListener( From 8e464a579b017b814e701c390794b394708655a0 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 27 Jun 2024 08:30:28 +0200 Subject: [PATCH 7/7] fix: review --- src/elements/clock/clock.ts | 170 +++++++++++++++++------------------- 1 file changed, 80 insertions(+), 90 deletions(-) diff --git a/src/elements/clock/clock.ts b/src/elements/clock/clock.ts index 1b15732fc5..9a76d1fa7d 100644 --- a/src/elements/clock/clock.ts +++ b/src/elements/clock/clock.ts @@ -86,20 +86,28 @@ export class SbbClockElement extends LitElement { /** Move the minutes hand every minute. */ private _handMovement?: ReturnType; - protected override willUpdate(changedProperties: PropertyValues): void { + protected override async willUpdate(changedProperties: PropertyValues): Promise { super.willUpdate(changedProperties); if (!isServer && changedProperties.has('now')) { - if (this.now) { - this._removeSecondsAnimationStyles(); - this._removeHoursAnimationStyles(); - this._stopClock(); - } else { - this._startClock(); - } + await this._startOrConfigureClock(); } } + protected override async firstUpdated(changedProperties: PropertyValues): Promise { + super.firstUpdated(changedProperties); + + if (!isServer) { + document.addEventListener('visibilitychange', this._handlePageVisibilityChange, false); + await this._startOrConfigureClock(); + } + } + + public override disconnectedCallback(): void { + super.disconnectedCallback(); + this._removeEventListeners(); + } + private _handlePageVisibilityChange = async (): Promise => { if (document.visibilityState === 'hidden') { await this._stopClock(); @@ -108,40 +116,51 @@ export class SbbClockElement extends LitElement { } }; - private _addEventListeners(): void { - document.addEventListener('visibilitychange', this._handlePageVisibilityChange, false); + private async _startOrConfigureClock(): Promise { + if (this.now) { + await this._stopClock(); + this._resetSecondsHandAnimation(); + this._setHandsStartingPosition(); + } else { + await this._startClock(); + } } - private _removeEventListeners(): void { - document?.removeEventListener('visibilitychange', this._handlePageVisibilityChange); - this._clockHandHours?.removeEventListener('animationend', this._moveHoursHandFn); - this._clockHandSeconds?.removeEventListener('animationend', this._moveMinutesHandFn); - clearInterval(this._handMovement); - } + /** Starts the clock by defining the hands starting position then starting the animations. */ + private async _startClock(): Promise { + this._clockHandHours?.addEventListener( + 'animationend', + this._moveHoursHandFn, + ADD_EVENT_LISTENER_OPTIONS, + ); + this._clockHandSeconds?.addEventListener( + 'animationend', + this._moveMinutesHandFn, + ADD_EVENT_LISTENER_OPTIONS, + ); - private _removeHoursAnimationStyles(): void { - this._clockHandHours?.classList.remove('sbb-clock__hand-hours--initial-hour'); - this.style.removeProperty('--sbb-clock-hours-animation-start-angle'); - this.style.removeProperty('--sbb-clock-hours-animation-duration'); - } + await new Promise(() => + setTimeout(() => { + this._setHandsStartingPosition(); - private _removeSecondsAnimationStyles(): void { - this._clockHandSeconds?.classList.remove('sbb-clock__hand-seconds--initial-minute'); - this._clockHandMinutes?.classList.remove('sbb-clock__hand-minutes--no-transition'); - this.style.removeProperty('--sbb-clock-seconds-animation-start-angle'); - this.style.removeProperty('--sbb-clock-seconds-animation-duration'); + this.style?.setProperty('--sbb-clock-animation-play-state', 'running'); + }, INITIAL_TIMEOUT_DURATION), + ); } - /** Given the current date, calculates the hh/mm/ss values and the hh/mm/ss left to the next midnight. */ - private _assignCurrentTime(): void { - const date = this.now ? null : new Date(); - const [hours, minutes, seconds] = date - ? [date.getHours(), date.getMinutes(), date.getSeconds()] - : this.now!.split(':').map((p) => +p); + /** Stops the clock by removing all the animations. */ + private async _stopClock(): Promise { + clearInterval(this._handMovement); - this._hours = hours % 12; - this._minutes = minutes; - this._seconds = seconds; + this._removeSecondsAnimationStyles(); + this._removeHoursAnimationStyles(); + + this._clockHandHours?.removeEventListener('animationend', this._moveHoursHandFn); + this._clockHandSeconds?.removeEventListener('animationend', this._moveMinutesHandFn); + + this._clockHandMinutes?.classList.add('sbb-clock__hand-minutes--no-transition'); + + this.style?.setProperty('--sbb-clock-animation-play-state', 'paused'); } /** Set the starting position for the three hands on the clock face. */ @@ -189,6 +208,18 @@ export class SbbClockElement extends LitElement { this.toggleAttribute('data-initialized', true); } + /** Given the current date, calculates the hh/mm/ss values and the hh/mm/ss left to the next midnight. */ + private _assignCurrentTime(): void { + const date = this.now ? null : new Date(); + const [hours, minutes, seconds] = date + ? [date.getHours(), date.getMinutes(), date.getSeconds()] + : this.now!.split(':').map((p) => +p); + + this._hours = hours % 12; + this._minutes = minutes; + this._seconds = seconds; + } + /** Set the starting position for the minutes hand. */ private _setMinutesHand(): void { this._clockHandMinutes?.style.setProperty( @@ -229,26 +260,6 @@ export class SbbClockElement extends LitElement { this._setMinutesHand(); } - /** Stops the clock by removing all the animations. */ - private async _stopClock(): Promise { - clearInterval(this._handMovement); - - this._removeSecondsAnimationStyles(); - this._removeHoursAnimationStyles(); - - this._clockHandHours?.removeEventListener('animationend', this._moveHoursHandFn); - this._clockHandSeconds?.removeEventListener('animationend', this._moveMinutesHandFn); - - this._clockHandMinutes?.classList.add('sbb-clock__hand-minutes--no-transition'); - - this.style?.setProperty('--sbb-clock-animation-play-state', 'paused'); - - if (this.now) { - this._resetSecondsHandAnimation(); - this._setHandsStartingPosition(); - } - } - /** * Removing animation by overriding with empty string, * then triggering a reflow and re add original animation by removing override. @@ -264,45 +275,24 @@ export class SbbClockElement extends LitElement { this._clockHandSeconds.style.removeProperty('animation'); } - /** Starts the clock by defining the hands starting position then starting the animations. */ - private async _startClock(): Promise { - this._clockHandHours?.addEventListener( - 'animationend', - this._moveHoursHandFn, - ADD_EVENT_LISTENER_OPTIONS, - ); - this._clockHandSeconds?.addEventListener( - 'animationend', - this._moveMinutesHandFn, - ADD_EVENT_LISTENER_OPTIONS, - ); - - await new Promise(() => - setTimeout(() => { - this._setHandsStartingPosition(); - - this.style?.setProperty('--sbb-clock-animation-play-state', 'running'); - }, INITIAL_TIMEOUT_DURATION), - ); + private _removeEventListeners(): void { + document?.removeEventListener('visibilitychange', this._handlePageVisibilityChange); + this._clockHandHours?.removeEventListener('animationend', this._moveHoursHandFn); + this._clockHandSeconds?.removeEventListener('animationend', this._moveMinutesHandFn); + clearInterval(this._handMovement); } - protected override async firstUpdated(changedProperties: PropertyValues): Promise { - super.firstUpdated(changedProperties); - - if (!isServer) { - this._addEventListeners(); - - if (this.now) { - await this._stopClock(); - } else { - await this._startClock(); - } - } + private _removeHoursAnimationStyles(): void { + this._clockHandHours?.classList.remove('sbb-clock__hand-hours--initial-hour'); + this.style.removeProperty('--sbb-clock-hours-animation-start-angle'); + this.style.removeProperty('--sbb-clock-hours-animation-duration'); } - public override disconnectedCallback(): void { - super.disconnectedCallback(); - this._removeEventListeners(); + private _removeSecondsAnimationStyles(): void { + this._clockHandSeconds?.classList.remove('sbb-clock__hand-seconds--initial-minute'); + this._clockHandMinutes?.classList.remove('sbb-clock__hand-minutes--no-transition'); + this.style.removeProperty('--sbb-clock-seconds-animation-start-angle'); + this.style.removeProperty('--sbb-clock-seconds-animation-duration'); } protected override render(): TemplateResult {