Supported Drawer Component (Android) #793
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Turned toolbar into action bar. If it's no first crumb then enable home as up - android puts back arrow and on press the navigation router navigates back
Used flex 1 for the drawer style so that it can be positioned anywhere. Tried it with absolute (like the react native one) but then it took up whole view - it should show in place so can put a drawer above or below the navigation bar, for example.
The drawer is modal so should block interaction with the content behind. Copied from react native ReactDrawerLayout https://github.com/facebook/react-native/blob/65a3259f039416394d2b945ec10565d38b3cad9e/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/drawer/ReactDrawerLayout.java#L67-L92 (dropped the try/catch because not sure what it's for)
Requested layout after attached to window. Without this the drawer appears again after the predictive back closes it. Not sure why but got the idea from the SceneView requesting layout on the React Native drawer otherwise it wouldn't open. Don't think they're the same issue?!
Matches the same code's position in coordinatorlayoutview
So it can be opened and closed declaratively
Raised the native event to sync properties
Need the js width to match the native width. The native drawerlayout adds a 64dp margin
The NavigationView inserts its own NavigationMenuView child (the content is meant to be children of this) and its receiving the events instead of the actual content. This menu view is the last child so it gets the events. Could implement ReactZIndexedViewGroup to stop it receiving them - but simpler to exclude it from the tree. Think it's to give some material styling - can't support that because React Native does the styling. Can support material styling when there's no actual child content (like in BottomNavigationView) but not for the drawer where there's actual content
Not sure of the best way to stop the NavigationView adding NavigationMenuView child. React Native always calls addView override with index so going with this approach
There's already an ActionBar component so actionBar is confusing. Plus not making the Toolbar an ActionBar, just hooking up the navigation (back or drawer)
Might want to do something when the drawer animation completes
Closed drawer using React Native back handling. Also removed default open value so it can work uncontrolled (same as active prop on SearchBar)
I checked back and this seemed to be for handling activities - apparently drawer didn't open on second activity (#266). This seems to be a good time to remove this old code
The ActionBarDrawerToggle handles connecting the toolbar to the drawer. Made weak references so that the scene view doesn't stop the toolbar and drawer from being disposed
No point using a custom icon with drawer toggle because the whole point is the animation. Plus it got messy because creating the toggle again when navigating back (onAttached called again which created the toggle) stopped the animation from working. It replaced the navigation icon with the one from the previous toggle. Makes most sense to use the built in icons when auto navigate. If user wants custom icon then can manually navigate
If drawer opened/closed during the navigation then the arrow wasn't sync'ed when navigating back
Turning on autoNavigation means let the Navigation router handle the navigation icon and navigation. Turning it off means user handles it. Keeps things simple - too complicated to allow autoNavigation with custom icon. Also clearer this way because there's only one navigation click listener which have to give to android when doing drawer toggle anyway.
On drawer scenario had custom navigationIcon and autoNavigation off. Then toggled autoNavigation to on then back to off and then custom icon didn't appear. That's because it hadn't changed - so stored it even when autoNavigation off so can restore it later
Need it even if setting because might turn auto navigation back on without changing the navigation icon
Copied over changes from toolbar to bottombar. This is just for the home/back - will do drawer toggle too though
Copied over drawer toggle from toolbar to bottombar
Main one was using pattern matching for instanceof
Improved the idea. Want a user to always turn on autoNavigation without having to think about whether it's first screen or not. Allowing a custom navigation icon is crucial for that as well as firing onNavigationPress (if it's not been taken over by auto navigation). So always going to set the navigation icon if it's provided. If it's not provided then going to use the default one from auto navigation (or from search bar if auto isn't on).
If there's on onNavigationPress then automatically handle navigation. Don't need an autoNavigation prop at all. There could be multiple toolbars - toolbar and bottombar - and don't want them both having hamburgers if there's a drawer. So only allowed one auto drawer handler in scene view. But same could happen for back arrow - in that case can add an onNavigationPress handler to the bottom bar and it won't have the back arrow. The one case think might need special prop is custom decorative nav icon on search bar on second scene and don't want back arrow (same for drawer). Can't add onNavigationPress because then not decorative anymore. Think this is super rare so can add prop if really needed later. Turning on by default is important because then users don't have to know about hamburger animation - they'll just get it if drawer and toolbar on same scene
It's the same as autoNavigation - true if there's no onNavigationPress and false otherwise - so don't need it anymore
Haven't tested them yet but needs to match the navigation bar
Not relevant now always setting the navigation icon. Also this code wouldn't work when doing the drawer toggle because that happens later so don't know at this stage what to do with the listener
Copied over drawer toggle from bottombar to searchbar. Had to copy the DrawerArrowDrawable setup from material SearchView because there's no public property to set to add it - the useDrawerArrowDrawable is how to set it but that can only be set from resources styles. Was gonna try using resource but the code is only a few lines long so copied it in instead. If don't add the drawerarrordrawable to the searchresults toolbar then android falls over because it goes down the FadeThroughDrawable path and DrawerArrowDrawable (on the searchbar) doesn't have constant state?!
Used navigationAccessibilityLabel prop instead - obviously better to use prop so user can control text. Tested can dynamically change prop when drawer open status changes
1. Always set the navigation icon. Can have auto back navigation with custom navigation icon. 2. Only set drawer if the crumb is 0. If drawer on second scene then only back arrow shows
Otherwise the arrow doesn't show up by default. Starts off either white or transparent and can't see it
Tried this color as the default tint color for the navigation bar and it was purple. Clearly wasn't being used because the hamburger was black. So removed for clarity
1. Always set the navigation icon. Can have auto back navigation with custom navigation icon. 2. Only set drawer if the crumb is 0. If drawer on second scene then only back arrow shows
The bottom bar has a default tint - maybe it's a theme thing. If gonna set one need to make sure it's restored too if the tintColor prop is removed
The default navigation icon tint is null - at least on the DayNight.NoActionBar material3 theme. This is confusing for new users because it looks like the image isn't there. So defaulted to black if null. It's not null on bottombar and searchbar so leaving them. Also switched to BlendModeColorFilter to avoid deprecation warning
Automatically refactored by android studio
Allowed unit tests to change the detent and only rendered the children if not hidden
Modelled on Sheet so allows user to fire the changeOpen to open and close it
There's no native drawer on ios so render children so it doesn't fall over
Don't know why it's different on fabric but it rendered very wrong
This happened when added the drawer open and close strings for the toggle. But since removed so don't want this showing up in the diff
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
DrawerComponent replaces React Native'sDrawerLayoutAndroid,Inherited from NavigationView to get predictive back (although there's a fix waiting on the next release of material components).
Hooked up the
NavigationBarto theDrawerto get the hamburger icon and toggle.Hooked up the
NavigationBarto theActionBarto get the arrow icon and back.