-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ALPHA-4956 Time Range Hidden #392
Conversation
const isTabBarHidden = !!mobileRoutes.find( | ||
(route) => | ||
route.type !== "fallback" && | ||
route.path === pathname && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are these 2 conditions neecessary, as opposed to just rely on the hideTabBar
prop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hideTabBar
Prop is not present in route options with type "fallback"
because, they do not have a path
prop to compare against and secondly are not the final route, they lead to other well defined route options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback routes are more less redirects. You should not need to hasndle the cases specifically since they would never display
I understand what you're trying to achieve. This works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please test this thoroughly because routing has gave us a lot of headaches. Make sure you deploy this branch on epsilon and test when installed and not.
@@ -41,95 +41,108 @@ const CustomNavTab: React.FC<{ | |||
</div> | |||
); | |||
|
|||
const RouterChild = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not only extract out the TabBar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please extract only the affected component, IonTabBar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have doubts but lgtm
This also removes the bottom Navbar for Notifications, User Settings and Auth Pages as per design
Before
After