added breakpoints logic #2
Conversation
| clearMediaQueryListenersForInteraction(interactionId: string): void { | ||
| this._removeMediaQueryListener(interactionId); | ||
| } |
There was a problem hiding this comment.
We can probably delete this one
| mql.removeEventListener('change', handler); | ||
| this.mediaQueryListeners.delete(interactionId); | ||
| } | ||
| this.clearMediaQueryListenersForInteraction(interactionId); |
There was a problem hiding this comment.
| this.clearMediaQueryListenersForInteraction(interactionId); | |
| this._removeMediaQueryListener(interactionId); |
packages/interact/src/core/add.ts
Outdated
| function _reconcile(root: IInteractElement, key: string): void { | ||
| remove(key); | ||
| add(root, key); | ||
| } |
There was a problem hiding this comment.
Let's make this function a static method of Interact, can be named update. This should solve the import problem
packages/interact/src/core/add.ts
Outdated
| id: string, | ||
| mql: MediaQueryList, | ||
| key: string, | ||
| handler: () => void, |
There was a problem hiding this comment.
Replace this param with sourceRoot, and the handler can be hard-coded here as interact.update(sourceRoot, sourceKey)
packages/interact/src/core/add.ts
Outdated
| return; | ||
| } | ||
|
|
||
| mql.addEventListener('change', handler); |
There was a problem hiding this comment.
| mql.addEventListener('change', handler); | |
| mql.addEventListener('change', () => Interact.update(sourceRoot, sourceKey)); |
packages/interact/src/core/add.ts
Outdated
| add(root, key); | ||
| } | ||
|
|
||
| function _setupMediaQueryListener( |
There was a problem hiding this comment.
Since all the logic here is tied to the instance, I suggest moving this function to be a method of the Interact instance itself. This should simplify everything, I think
| let matchMediaMock: ReturnType<typeof vi.fn>; | ||
|
|
||
| // Helper to create mock MediaQueryList objects | ||
| const createMockMQL = (query: string, initialMatches = false) => { |
There was a problem hiding this comment.
This mock already exists at the top.
Also, since we're running in JSDOM env we may be able to remove some of the mocks
| matchMediaMock = vi.fn().mockImplementation((query: string) => createMockMQL(query)); | ||
|
|
||
| Object.defineProperty(window, 'matchMedia', { | ||
| writable: true, | ||
| configurable: true, | ||
| value: matchMediaMock, | ||
| }); |
There was a problem hiding this comment.
Already has a mock at the top, so first thing would be to reuse that.
| listenerEntry!.handler(mockEvent); | ||
|
|
||
| // Verify reconcile was triggered (clearInteractionStateForKey is called during reconcile) | ||
| expect(clearStateSpy).toHaveBeenCalledWith('responsive-button'); |
There was a problem hiding this comment.
You actually have a spy on a method that's a part of the inner implementation.
If we're doing a unit test it's better to check that the top level function that's in charge of this logic to be invoked. This case the _reconcile function itself. If this becomes a method of Interact it will be more testable and we can observe that directly.
| }; | ||
|
|
||
| beforeEach(() => { | ||
| Interact.destroy(); |
There was a problem hiding this comment.
Shouldn't be necessary
| Interact.destroy(); |
No description provided.