Update displayed position in statusbar when flycam changes

[MichaelBuessemeyer]

Currently, the displayed position of the mouse in the statusbar does only update, when the mouse is moved. This PR now also changes the statusbar, so that the position also updates, each time the flycam changes (e.g. via space in z direction).

URL of deployed dev instance (used for testing):

• https://___.webknossos.xyz

Steps to test:

• Open a tracing and move the mouse over a segment.
• Now move around with space and the arrow keys.
• The displayed position should always update.

Issues:

• fixes Scrolling data in z should update status bar info #5443

---

• [ ] Updated (unreleased) changelog
• [ ] Updated (unreleased) migration guide if applicable
• [ ] Updated documentation if applicable
• [ ] Adapted wk-connect if datastore API changes
• [ ] Needs datastore update after deployment
• Ready for review

[philippotto]

Cool, short fix :) However, I think, there's a more robust way to tackle this problem. See my other comment 🤓

[philippotto]

I think, this line does not fix the problem at the root. Apart from the fact, that this line purely exists for its side effect (which isn't really idiomatic in this react context), I think, there can still be scenarios where the mouse position will be wrong (didn't test it, but for example, going to full screen with . might not update the position).

I think, the actual problem is that line 286 uses Store.getState() which we should get rid of to be fully reactive. A simple solution could be to use something like const globalMousePosition = useSelector(state => calculateGlobalPos(state), { x, y }).

I think, there is one problem left with my suggested solution: calculateGlobalPos will always return a new instance which means that the statusbar will re-render consequently. However, we've got a helper function for this: reuseInstanceOnEquality. You can read its comment to learn more about it. I think, you can use that helper to make usages of calculateGlobalPos more efficient (since the returned instances will be re-used if they don't change).

Something like
export const calculateGlobalPos = reuseInstanceOnEquality(_calculateGlobalPos); in view_mode_accessor.js. Let me know if you have questions!

[MichaelBuessemeyer]

I am a bit confused right now. According to the docs shown by vscode we do not need the helper function reuseInstanceOnEquality as useSelector has a second argument:

A hook to access the redux store's state. This hook takes a selector function as an argument. The selector is called with the store state.
This hook takes an optional equality comparison function as the second parameter that allows you to customize the way the selected state is compared to determine whether the component needs to be re-rendered.
If you do not want to have to specify the root state type for whenever you use this hook with an inline selector you can use the TypedUseSelectorHook interface to create a version of this hook that is properly typed for your root state.
@param selector — the selector function
@param equalityFn — the function that will be used to determine equality
@returns — the selected state

The online docs do say the same.

But when I tried this out, the two values passed to the comparison function were always the same 😕.

Thus I simply implemented your suggestion.

I additionally found that the mapping settings view updated with each mouse movement. A quick look revealed that it subscribed to the mouse position without using it. Thus I just removed this in this pr.

[philippotto]

I am a bit confused right now. According to the docs shown by vscode we do not need the helper function reuseInstanceOnEquality as useSelector has a second argument:

Yes, this is also a possible workaround. However, that approach requires that all callers of useSelector indeed use the equalityFn parameter (which is very easy to forget and hard to notice afterwards). Using reuseInstanceOnEquality avoids that requirement.

But when I tried this out, the two values passed to the comparison function were always the same confused.

Hm, probably not worth to investigate :)

Thus I simply implemented your suggestion.

👍

I additionally found that the mapping settings view updated with each mouse movement. A quick look revealed that it subscribed to the mouse position without using it. Thus I just removed this in this pr.

Great 👍 This probably dates back to earlier versions where a segment id table was rendered in the mapping view.

[philippotto]

Awesome! Works very well 🎉

[philippotto]

please update the changelog before merging