TecKnow / muster-tools

A package for assigning players to tables at multi-table, walk-in gaming events.
GNU General Public License v3.0
0 stars 0 forks source link

Should the value of the BottomNavigation component be derived from the URL? #13

Closed TecKnow closed 3 years ago

TecKnow commented 3 years ago

Current the BottomNavigation component sets its own value using an onChange handler. This is the 0-based index of the selected item. This works for now, but I worry it could go out of sync. The Egghead.io redux tutorials included a way to set links to active or inactive based on the URL. Should I use something like that? If so, I need to re-learn how.

TecKnow commented 3 years ago

The BottomNavigation component of @material-ui accepts a value prop indicating which of its children should be highlighted. When using react-router the URL and the highlighted value in the <BottomNavigation> component can go out of sync. This happens when the user manually alters the URL, for example.

The specialized <Link> component from react-router does not help because this isn't an issue that can be fixed by styling a link.

TecKnow commented 3 years ago

To resolve this issue a react component called <RouterBottomNavigation> was created. As in the base component, its children should be an array of <BottomNavigationAction> components with to properties expressing their destination URL.

<RouterBottomNavigation> uses the useLocation() hook from react-router-dom to access the current URL. It iterates over its children to find the index of of the child whose destination matches the end of the current URL. It uses that index as its value, causing the matching <BottomNavigationAction> to be highlighted based on the URL and kept in sync.

The only thing about this approach that is unclear is the safe and idiomatic way to iterate over a component's children. RouterBottomNavigation currently uses React.Children.toArray() but the react documentation around this issue is not clear.

I can't figure out how to reach the documentation explaining that this is the safest way from the main documentation, so I cannot be sure it is current.