SupremeTechnopriest / react-idle-timer

User activity timer component
https://idletimer.dev
MIT License
1.14k stars 143 forks source link

🐞 IdleTimer return functions like `isPrompted`, `isIdle` should be boolean state vars, not () => boolean functions #396

Open donp-usbank opened 5 days ago

donp-usbank commented 5 days ago

What happened?

Trying to control an "Extend session" prompt dialog using isPrompted.

In React, using a Dialog component with an isOpen prop requires use of a React state variable so that the component will re-render when the state (isPrompted) is changing. However, since idle-timer provides only a function, and not a hook-based boolean state, we can't detect when the state is changing, and therefore must use our own managed state value which is set by the onActive, onIdle, and onPrompt event handlers. This makes the isPrompted function redundant and not useful.

This could be made more useful by including and managing these states internally in the useIdletimer hook and returning them along with the other return values/objects.

Reproduction Steps

1. Implement a session confirm prompt per example https://idletimer.dev/docs/features/confirm-prompt
2. Observe that although `isPrompted` function is provided by the idle-timer library, it's not used and wouldn't be useful.
3. Observe that additional state must be implemented as boilerplate in order to have a state that represents the prompted state.
...

Relevant log output

No response

Screenshots or Additional Context

No response

Module Version

5.7.2

What browsers are you seeing the problem on? Select all that apply.

No response

What devices are you seeing the problem on?

No response

Verification

SupremeTechnopriest commented 4 days ago

This would be a breaking change and will have to wait for a major version release. Have you considered using onPresenceChange? I think that will get you what you want.

https://idletimer.dev/docs/api/props#onpresencechange

donp-usbank commented 4 days ago

Thanks @SupremeTechnopriest

Have you considered using onPresenceChange? I think that will get you what you want.

I did look at this, and no this won't get us what we want for the same reasons noted above. Even in the example for onPresenceChange, this requires boilerplate logic implementation to produce a result variable like isPrompted - but that logic is based on rules defined by the library; meaning, the fact that the meaning of isPrompted is that presence.type is 'active' and presence.prompted is true. So, 1. the fact that users of the lib would need to implement this logic which seems to be a concern of the library, and 2. the fact that these vars are still not state variables that will cause a component to re-render - means that this isn't a useful solution for displaying a confirmation prompt.

The suggestion here is to modify the library to produce true React boolean state variables as part of the return value of the useIdleTimer hook. That could be done in either a breaking way by changing the isPrompted return property from a function to a boolean, or in a non-breaking way by adding a new prompted boolean return state variable.

Either way, a true state variable is what's useful, for the reasons above.

Thanks.