accept `focusDetails` argument in `focusSelf`
An application scrolls to an element in onFocus handler. onFocus can be called either automatically (for example, on page load) or in response of user action (keyboard event or, in case of webOS, mouse event). For the sake of convenience, cursor hover on webOS is handled by calling focusSelf
Because of UX, scroll on automatic focus has behavior: 'instant', on user action — behavior: 'smooth'. But because there's no way to pass MouseEvent to focusSelf, there's no way to distinguish "mouse hover focus" from "page load focus". Accepting focusDetails argument in focusSelf would allow application to pass MouseEvent when it's necessary to do that and then do something like this:
const onFocus = (layout, extraProps, focusDetails) => {
scrollIntoViewIfNeeded(layout.node, {
behavior:
focusDetails?.event instanceof KeyboardEvent ||
focusDetails?.event instanceof MouseEvent
? 'smooth'
: 'instant',
});
};
This change isn't critical, since application can call setFocus(focusKey, { event }) instead of focusSelf()/focusSelf({ event }), but a shorter option is convenient to have
While setFocus accepts focusDetails argument, it is missing in type definition. Is it on purpose?
Hello @zemlanin,
Thanks a lot for the contribution, it's appreciated.
We'll have a look, discuss and get back to you.
Cheers
Hello again @zemlanin,
We will be reviewing this PR next week.
From a quick glance it looks like this will need another small change related to focusDetails.event, since currently it is always of type KeyboardEvent.
We might just do this change ourselves, depending on what comes out of the review, but we'll get back to you.
Thanks regardless for the contribution! 🙇
I didn't want to potentially require library users to deal with MouseEvent type if they don't need it (and since the library itself doesn't fire them) and extended the focusDetails type in the app code
I created a new PR based on your suggestion @zemlanin . And this change was published in the last release.
Now focusDetails can contain a KeyboardEvent or MouseEvent, or any value you pass into it. In case you receive a SyntheticEvent you might need to use the nativeEvent key (instead of event). So your previous code should be slightly changed to:
const onFocus = (layout, extraProps, focusDetails) => {
scrollIntoViewIfNeeded(layout.node, {
behavior:
focusDetails?.event instanceof KeyboardEvent ||
focusDetails?.nativeEvent instanceof MouseEvent
? 'smooth'
: 'instant',
});
};
Alternatively you can pass any object when you load your content/page, for instance:
useEffect(() => {
focusSelf({scroll: 'instant'});
}, [focusSelf]);
And then previous example code can be replaced to:
const onFocus = (layout, extraProps, focusDetails) => {
scrollIntoViewIfNeeded(layout.node, {
behavior:
focusDetails?.scroll === 'instant'
? 'instant'
: 'smooth',
});
};
Thanks for your contribution.
Closing as fixed.