From 597e79d7fc12737353c8c9eb3f6e77ef0a04e9ed Mon Sep 17 00:00:00 2001 From: Christian Pillsbury Date: Fri, 12 Sep 2025 15:48:54 -0700 Subject: [PATCH] fix(time-display): clean up time utilities and simplify components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove deprecated formatDuration function from time utils - Simplify HTML current time display by removing template generation - Use direct shadow DOM text content updates for better performance - Remove unused imports from React current time display component - Enable show-remaining by default in HTML skin - Ensure consistent negative time formatting across platforms 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- packages/core/media-store/src/utils/time.ts | 55 ++++++------------- .../components/media-current-time-display.ts | 44 +++------------ .../html/html/src/skins/media-skin-default.ts | 2 +- .../src/components/CurrentTimeDisplay.tsx | 25 +++------ 4 files changed, 35 insertions(+), 91 deletions(-) diff --git a/packages/core/media-store/src/utils/time.ts b/packages/core/media-store/src/utils/time.ts index ee059157..07f92f28 100644 --- a/packages/core/media-store/src/utils/time.ts +++ b/packages/core/media-store/src/utils/time.ts @@ -1,6 +1,6 @@ /** * @fileoverview Time formatting utilities for media components - * + * * This module provides utilities for formatting time values in various formats * suitable for display in media player UI components. Based on patterns from * Media Chrome but adapted for VJS-10 architecture. @@ -19,7 +19,7 @@ const UnitLabels = [ plural: 'hours', }, { - singular: 'minute', + singular: 'minute', plural: 'minutes', }, { @@ -41,7 +41,7 @@ const toTimeUnitPhrase = (timeUnitValue: number, unitIndex: number): string => { * Converts numeric seconds into a human-readable phrase for accessibility * @param seconds - A (positive or negative) time, represented as seconds * @returns The time, represented as a phrase of hours, minutes, and seconds - * + * * @example * formatAsTimePhrase(3661) // "1 hour, 1 minute, 1 second" * formatAsTimePhrase(90) // "1 minute, 30 seconds" @@ -49,13 +49,13 @@ const toTimeUnitPhrase = (timeUnitValue: number, unitIndex: number): string => { */ export function formatAsTimePhrase(seconds: number): string { if (!isValidNumber(seconds)) return ''; - + const positiveSeconds = Math.abs(seconds); const negative = positiveSeconds !== seconds; const secondsDateTime = new Date(0, 0, 0, 0, 0, positiveSeconds, 0); const timeParts = [ secondsDateTime.getHours(), - secondsDateTime.getMinutes(), + secondsDateTime.getMinutes(), secondsDateTime.getSeconds(), ]; @@ -63,7 +63,7 @@ export function formatAsTimePhrase(seconds: number): string { // Convert non-0 values to a string of the value plus its unit .map( (timeUnitValue, index) => - timeUnitValue && toTimeUnitPhrase(timeUnitValue, index) + timeUnitValue && toTimeUnitPhrase(timeUnitValue, index), ) // Ignore/exclude any 0 values .filter((x) => x) @@ -77,15 +77,15 @@ export function formatAsTimePhrase(seconds: number): string { } /** - * Converts a time, in numeric seconds, to a formatted string representation - * of the form [HH:[MM:]]SS, where hours and minutes are optional, either + * Converts a time, in numeric seconds, to a formatted string representation + * of the form [HH:[MM:]]SS, where hours and minutes are optional, either * based on the value of `seconds` or (optionally) based on the value of `guide`. * * @param seconds - The total time you'd like formatted, in seconds - * @param guide - A number in seconds that represents how many units you'd want + * @param guide - A number in seconds that represents how many units you'd want * to show. This ensures consistent formatting between e.g. 35s and 4834s. * @returns A string representation of the time, with expected units - * + * * @example * formatTime(90) // "1:30" * formatTime(3661) // "1:01:01" @@ -96,7 +96,7 @@ export function formatAsTimePhrase(seconds: number): string { export function formatTime(seconds: number, guide?: number): string { // Handle negative values let negative = false; - + if (seconds < 0) { negative = true; seconds = 0 - seconds; @@ -107,7 +107,7 @@ export function formatTime(seconds: number, guide?: number): string { let s: number | string = Math.floor(seconds % 60); let m: number | string = Math.floor((seconds / 60) % 60); let h: number | string = Math.floor(seconds / 3600); - + const gm = guide ? Math.floor((guide / 60) % 60) : 0; const gh = guide ? Math.floor(guide / 3600) : 0; @@ -124,7 +124,8 @@ export function formatTime(seconds: number, guide?: number): string { // If hours are showing, we may need to add a leading zero. // Always show at least one digit of minutes. - const minutesString = ((showHours || gm >= 10) && (m as number) < 10 ? '0' + m : m) + ':'; + const minutesString = + ((showHours || gm >= 10) && (m as number) < 10 ? '0' + m : m) + ':'; // Check if leading zero is needed for seconds const secondsString = (s as number) < 10 ? '0' + s : s; @@ -132,15 +133,6 @@ export function formatTime(seconds: number, guide?: number): string { return (negative ? '-' : '') + hoursString + minutesString + secondsString; } -/** - * Checks if a duration value should be considered valid for display - * @param duration - The duration value to check - * @returns True if the duration is a valid, displayable number - */ -export function isValidDuration(duration: unknown): duration is number { - return isValidNumber(duration) && duration >= 0; -} - /** * Formats a time value with fallback handling for invalid values * @param time - The time value to format in seconds (duration, currentTime, etc.) @@ -149,23 +141,12 @@ export function isValidDuration(duration: unknown): duration is number { * @returns Formatted time string or fallback */ export function formatDisplayTime( - time: unknown, - guide?: number, - fallback: string = '--:--' + time: unknown, + guide?: number, + fallback: string = '--:--', ): string { - if (!isValidDuration(time)) { + if (!isValidNumber(time)) { return fallback; } return formatTime(time, guide); } - -/** - * @deprecated Use formatDisplayTime instead. Will be removed in a future version. - */ -export function formatDuration( - duration: unknown, - guide?: number, - fallback: string = '--:--' -): string { - return formatDisplayTime(duration, guide, fallback); -} \ No newline at end of file diff --git a/packages/html/html/src/components/media-current-time-display.ts b/packages/html/html/src/components/media-current-time-display.ts index afdead26..d3ef7703 100644 --- a/packages/html/html/src/components/media-current-time-display.ts +++ b/packages/html/html/src/components/media-current-time-display.ts @@ -7,23 +7,11 @@ import { currentTimeDisplayStateDefinition, formatDisplayTime, } from '@vjs-10/media-store'; -import { namedNodeMapToObject } from '../utils/element-utils.js'; - -export function getTemplateHTML( - this: typeof CurrentTimeDisplayBase, - _attrs: Record, - _props: Record = {}, -) { - return /* html */ ` - - `; -} export class CurrentTimeDisplayBase extends HTMLElement { static shadowRootOptions = { mode: 'open' as ShadowRootMode, }; - static getTemplateHTML = getTemplateHTML; static observedAttributes = ['show-remaining']; _state: @@ -40,15 +28,6 @@ export class CurrentTimeDisplayBase extends HTMLElement { this.attachShadow( (this.constructor as typeof CurrentTimeDisplayBase).shadowRootOptions, ); - - const attrs = namedNodeMapToObject(this.attributes); - const html = ( - this.constructor as typeof CurrentTimeDisplayBase - ).getTemplateHTML(attrs); - const shadowRoot = this.shadowRoot as unknown as ShadowRoot; - shadowRoot.setHTMLUnsafe - ? shadowRoot.setHTMLUnsafe(html) - : (shadowRoot.innerHTML = html); } } @@ -78,21 +57,14 @@ export class CurrentTimeDisplayBase extends HTMLElement { _update(_props: any, state: any) { this._state = state; - // Update the span content with formatted time - const spanElement = this.shadowRoot?.querySelector('span') as HTMLElement; - if (spanElement) { - if ( - this.showRemaining && - state.duration != null && - state.currentTime != null - ) { - // Show remaining time: duration - currentTime - const remainingTime = state.duration - state.currentTime; - spanElement.textContent = `-${formatDisplayTime(remainingTime)}`; - } else { - // Show current time (default behavior) - spanElement.textContent = formatDisplayTime(state.currentTime); - } + /** @TODO Should this live here or elsewhere? (CJP) */ + const timeLabel = + this.showRemaining && state.duration != null && state.currentTime != null + ? formatDisplayTime(-(state.duration - state.currentTime)) + : formatDisplayTime(state.currentTime); + + if (this.shadowRoot) { + this.shadowRoot.textContent = timeLabel; } } } diff --git a/packages/html/html/src/skins/media-skin-default.ts b/packages/html/html/src/skins/media-skin-default.ts index 02198c7a..2ac99d04 100644 --- a/packages/html/html/src/skins/media-skin-default.ts +++ b/packages/html/html/src/skins/media-skin-default.ts @@ -115,7 +115,7 @@ export function getTemplateHTML() { - + diff --git a/packages/react/react/src/components/CurrentTimeDisplay.tsx b/packages/react/react/src/components/CurrentTimeDisplay.tsx index 16455a38..b7a54502 100644 --- a/packages/react/react/src/components/CurrentTimeDisplay.tsx +++ b/packages/react/react/src/components/CurrentTimeDisplay.tsx @@ -1,8 +1,4 @@ -import { - shallowEqual, - useMediaSelector, - useMediaStore, -} from '@vjs-10/react-media-store'; +import { shallowEqual, useMediaSelector } from '@vjs-10/react-media-store'; import * as React from 'react'; import { toConnectedComponent } from '../utils/component-factory'; import { @@ -11,7 +7,6 @@ import { } from '@vjs-10/media-store'; export const useCurrentTimeDisplayState = (_props: any) => { - const mediaStore = useMediaStore(); /** @TODO Fix type issues with hooks (CJP) */ const mediaState = useMediaSelector( currentTimeDisplayStateDefinition.stateTransform, @@ -30,7 +25,7 @@ export type CurrentTimeDisplayState = ReturnType; export const useCurrentTimeDisplayProps = ( props: React.PropsWithChildren<{ showRemaining?: boolean; [k: string]: any }>, - state: ReturnType, + _state: ReturnType, ) => { const baseProps: Record = { /** external props spread last to allow for overriding */ @@ -49,17 +44,13 @@ export const renderCurrentTimeDisplay = ( ) => { const { showRemaining, ...restProps } = props; - let timeToDisplay: number | undefined; + /** @TODO Should this live here or elsewhere? (CJP) */ + const timeLabel = + showRemaining && state.duration != null && state.currentTime != null + ? formatDisplayTime(-(state.duration - state.currentTime)) + : formatDisplayTime(state.currentTime); - if (showRemaining && state.duration != null && state.currentTime != null) { - // Show remaining time: duration - currentTime - timeToDisplay = state.duration - state.currentTime; - } else { - // Show current time (default behavior) - timeToDisplay = state.currentTime; - } - - return -{formatDisplayTime(timeToDisplay)}; + return {timeLabel}; }; export type renderCurrentTimeDisplay = typeof renderCurrentTimeDisplay;