feat: implement subscribeKeyboardHeight and useKeyboardHeight function #4
Conversation
| vi.mock('@webview-kit/core', () => ({ | ||
| subscribeKeyboardHeight: vi.fn(), | ||
| })); |
There was a problem hiding this comment.
This mock path @webview-kit/core doesn't match the actual import path used in useKeyboardHeight.ts 🤣
| }: SubscribeKeyboardHeightOptions): SubscribeKeyboardHeightResult { | ||
| const handler = () => callback(getKeyboardHeight()); | ||
|
|
||
| const visualViewport = window.visualViewport; |
There was a problem hiding this comment.
I think this will throw ReferenceError: window is not defined in SSR environments (Next.js, Remix, etc.)
| visualViewport.addEventListener('resize', handler); | ||
| visualViewport.addEventListener('scroll', handler); |
There was a problem hiding this comment.
The resize and scroll events can fire dozens or even hundreds of times per second. When the keyboard appears with animation, this could lead to unnecessary re-renders or performance issues.
I can think of a few possible approaches here, but it might also be reasonable to treat optimization as a follow-up.
• Provide an optional throttle configuration.
• Wrap the handler with requestAnimationFrame by default.
• Expose a throttle option for more fine-grained control.
There was a problem hiding this comment.
Since resize and scroll can fire almost simultaneously, the callback may be called twice with the same value. 🤔
I was just thinking out loud.... maybe we could keep the last height in a small internal state and skip the callback when there’s no change?
There was a problem hiding this comment.
thanks for the feedback! I applied the throttle logic and also used a variable to detect if the height has changed from the previous one, in order to block calling multiple callback function 😄
There was a problem hiding this comment.
similar to the comment about duplicate callbacks, it would be nice to add a test for this as well.
There currently isn’t a test that verifies whether the callback is invoked multiple times when resize and scroll fire at the same time.
There was a problem hiding this comment.
Just an additional idea — what do you think about adding a test case that simulates rapid event firing?
For example, something like this to ensure the callback isn’t called excessively when events occur in quick
// Simulate rapid events
for (let i = 0; i < 100; i++) {
resizeHandler?.();
}There was a problem hiding this comment.
applied test codes, referencing your useScrollDirection hook!
| [immediate] | ||
| ); | ||
|
|
||
| return keyboardHeight; |
There was a problem hiding this comment.
👞
| return keyboardHeight; | |
| return { keyboardHeight }; |
zztnrudzz13
force-pushed
the
feat/subscribe-keyboard-height
branch
from
e1b38e7 to
7fef09e
Compare
Overview
This PR adds the subscribeKeyboardHeight() utility function to react-simplikit, which provides a reactive subscription pattern for listening to on-screen keyboard height changes.
Checklist
yarn run fixto format and lint the code and docs?yarn run test:coverageto make sure there is no uncovered line?