HomePhabricator
Diffusion Comm 6b3f48e43038

[web] Fix a bug where incorrect navigation state prop was used in navigation…

Description

[web] Fix a bug where incorrect navigation state prop was used in navigation panel

Summary:
This bug manifested itself when adding danger zone - the second navigation item on settings page. The issue was that we were always using state.navInfo.tab to determine which navigation item should be highlighted. This approach worked correctly in app switcher and appeared to work correctly in settings switcher - but only because there was only one item.

The solution to this issue is to allow deciding which part of the state should be used. For the settings switcher the correct state is located at state.navInfo.settingsSection. It is implemented by adding a new prop to NavigationPanel.Container which is a function that takes the AppState and returns the current nav panel.

This works great and is easy to use but has one big issue. We no longer can safely type NavigationPanelItemProps as tab prop now depends on tabSelector return value. From high level, what we want is to have a parent-children relationship where both parent and children are generic components using the same type parameter. I tried solving that https://flow.org/try/#0JYWwDg9gTgLgBAKjgQwM5wEoFNkGN4BmUEIcARFDvmQNwBQdMAnmFnAIJhgDKMyMbALxwA3gF96jFmwByyAG7AA5v2AQAdgAVk6rABsAkgJCbiYVAB4AKgD44wkXThwA1HwBGALjhWANE9dcAAtgPQATSnVvbDwYADoZCDCsfwkGAgBXdXw1dTg5RRUYXO1dQ2NrGwAKALAzVG8C5VUNUv0jLBN6yv8ASmiqeMTk0QDcDVR4EThg0IisPLF7ODqIc3pnShgMqDzZ8Mj6MQZmVnyFZuLWnX0AYQ0+YF0oUzXLW2XHZzdkd259LD4aDeTg8PgCex2PwBFz7eZRTCDOK3EIHBbsKBQZBMCwAfhi+DiAFE9J0FjALKcsBACOdCi0tDdyp1KjYbKlJAB6TlwW46dQQeBKLDwOFxVbmOIeODuQHIDKoNgSrCwJhwaXAdAgTWoJ5KOBPODagAeWDCUjOTSKJSZ93Uj2er3MACZKp8YR5-qSgVAQVxePwhFD-N84ZEBrFkaj4RisTj8UiSWT7RYAs4CfF7uANOSrNILFaGW1mV03qyQ3A2Ry6EA but without success. I also found an issue https://github.com/facebook/flow/issues/8582 that indicates that other people have the same problem.

For now, I decided that using string is better than having generic NavigationPanelItem<T> with typeof NavigationPanelItem because that would effectively mean we're using any.

Test Plan: Open chat and switch apps a couple of times - every time the current tab should be highlighted. Open settings http://localhost/comm/settings/account/ and verify that the account tab is highlighted.

Reviewers: def-au1t, atul, benschac, ashoat

Reviewed By: def-au1t, atul, ashoat

Subscribers: ashoat, Adrian, yayabosh

Differential Revision: https://phabricator.ashoat.com/D4091