fix: add popover core, use in html and improve factory (#204)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

packages/html/src/utils/component-factory.ts

@@ -1,16 +1,18 @@
 import type { MediaStore } from '@videojs/core/store';
 import { ConsumerMixin } from '@open-wc/context-protocol';
+import { shallowEqual, toCamelCase } from '@videojs/utils';
+import { setAttributes } from '@videojs/utils/dom';
 
 /**
  * Generic types for HTML component hooks pattern
  * Mirrors the React hooks architecture for consistency
  */
-export type StateHook<T = any> = (mediaStore: MediaStore) => T;
+export type StateHook<E extends HTMLElement, T = any> = (element: E, mediaStore: MediaStore) => T;
 
-export type PropsHook<T = any, P = any> = (state: T, element: HTMLElement) => P;
+export type PropsHook<E extends HTMLElement, T = any, P = any> = (element: E, state: T) => P;
 
-export interface ConnectedComponentConstructor<State> {
-  new (state: State): HTMLElement;
+export interface ConnectedComponentConstructor<E extends HTMLElement, State> {
+  new (state: State): E;
 }
 
 let currentCoreInstances: any[] = [];
@@ -25,16 +27,15 @@ let currentCoreIndex: number = 0;
  * @param BaseClass - Base custom element class to extend
  * @param stateHook - Hook that defines state keys and transformation logic
  * @param propsHook - Hook that handles element attributes and properties based on state
- * @param eventsHook - Hook that defines event handling logic
  * @param displayName - Display name for debugging
  * @returns Connected custom element class with media store integration
  */
-export function toConnectedHTMLComponent<State = any>(
+export function toConnectedHTMLComponent<E extends HTMLElement, State = any>(
   BaseClass: CustomElementConstructor,
-  stateHook: StateHook<State> | undefined,
-  propsHook: PropsHook<State>,
+  stateHook: StateHook<E, State> | undefined,
+  propsHook: PropsHook<E, State>,
   displayName?: string,
-): ConnectedComponentConstructor<State> {
+): ConnectedComponentConstructor<E, State> {
   const ConnectedComponent = class extends ConsumerMixin(BaseClass) {
     static get observedAttributes(): string[] {
       return [
@@ -43,42 +44,52 @@ export function toConnectedHTMLComponent<State = any>(
       ];
     }
 
-    _mediaStore: any;
-    _coreInstances = [];
+    _state: State | undefined;
+    #mediaStore: MediaStore | undefined;
+    #coreInstances: { core: any; listening: boolean }[] = [];
 
     contexts = {
       mediaStore: (mediaStore: any) => {
-        this._mediaStore = mediaStore;
+        this.#mediaStore = mediaStore;
 
         // Subscribe to media store state changes
-        // Split into two phases: state transformation, then props update
-        this._mediaStore.subscribe(() => {
-          currentCoreIndex = 0;
-          currentCoreInstances = this._coreInstances;
+        mediaStore.subscribe(() => {
+          this.#render();
 
-          // Phase 1: Transform raw media store state (state concern)
-          const state = stateHook?.(mediaStore) ?? mediaStore.getState();
-          // Phase 2: Update element attributes/properties (props concern)
-          const props = propsHook(state ?? {} as State, this);
-
-          // @ts-expect-error any
-          this._update(props, state, mediaStore);
-
-          for (const instance of currentCoreInstances) {
+          for (const instance of this.#coreInstances) {
             if (!instance.listening) {
               instance.listening = true;
-              instance.core.subscribe(() => {
-                const state = instance.core.getState();
-                const props = propsHook(state ?? {} as State, this);
-                // @ts-expect-error any
-                this._update(props, state, mediaStore);
-              });
+              instance.core.subscribe(this.#render);
             }
           }
         });
       },
     };
 
+    #render = (): void => {
+      if (!this.#mediaStore) return;
+
+      currentCoreIndex = 0;
+      currentCoreInstances = this.#coreInstances;
+
+      // Split into two phases: state transformation, then props update
+      const state = stateHook?.(this as unknown as E, this.#mediaStore);
+      const props = propsHook(this as unknown as E, state ?? {} as State);
+      this._update(props, state, this.#mediaStore);
+    };
+
+    _update(props: any, state: State | undefined, _mediaStore: MediaStore): void {
+      this._state = state;
+      // @ts-expect-error any
+      super._update?.(props, state, _mediaStore);
+      setAttributes(this, props);
+    }
+
+    attributeChangedCallback(name: string, oldValue: string, newValue: string): void {
+      super.attributeChangedCallback?.(name, oldValue, newValue);
+      this.#render();
+    }
+
     connectedCallback(): void {
       super.connectedCallback?.();
     }
@@ -98,7 +109,7 @@ export function toConnectedHTMLComponent<State = any>(
     Object.defineProperty(ConnectedComponent, 'name', { value: displayName });
   }
 
-  return ConnectedComponent;
+  return ConnectedComponent as unknown as ConnectedComponentConstructor<E, State>;
 }
 
 export function getCoreState<T extends {
@@ -114,6 +125,24 @@ export function getCoreState<T extends {
 
   currentCoreIndex++;
 
-  core.setState(state);
+  const coreState = core.getState();
+  const oldState: Record<string, any> = {};
+  for (const key in state) {
+    oldState[key] = coreState[key];
+  }
+  // Only set the state if it has changed
+  if (!shallowEqual(oldState, state)) {
+    core.setState(state);
+  }
+
   return core.getState();
 }
+
+export function getPropsFromAttrs(element: HTMLElement): Record<string, any> {
+  const props: Record<string, any> = {};
+  for (const attr of element.attributes) {
+    const camelName = toCamelCase(attr.name);
+    props[camelName] = element[camelName as keyof typeof element];
+  }
+  return props;
+}