Skip to content
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

Hide mobile nav on subpages based on backPath #4695

Merged
merged 3 commits into from Sep 24, 2022

Conversation

hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Sep 24, 2022

  • Hide Mobile Nav on subpages with backPath
  • Add backPath at following places. It was missing there
    • Workflow Edit
  • Implement Back Button using backPath approach for App Store as well. It solves inconsistency b/w backPath plus Heading combination of AppStore and other pages.

It covers all the cases mentioned here #4398 (comment)

I think we need a back button on mobile as well. It is more important to be there because navigation is gone there.
Edit: Found out that it was supposed to work already, but wasn't working due to a bug. Fixed that bug as well.

This is how the flow would be to explore different apps.
Apps -> App1 -> Logo Click -> Apps -> App2
Apps -> App1 -> Back-> App2

Loom Demo

What does this PR do?

Fixes #4398

Environment:Production

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

See loom video

@vercel
Copy link

vercel bot commented Sep 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Sep 24, 2022 at 11:52AM (UTC)

@hariombalhara hariombalhara changed the title Hide nav on subpages based on backPath. Add backPath where it was mis… Hide nav on subpages based on backPath Sep 24, 2022
@hariombalhara hariombalhara changed the title Hide nav on subpages based on backPath Hide mobile nav on subpages based on backPath Sep 24, 2022
@@ -66,225 +64,213 @@ const Component = ({
const allowedMultipleInstalls = categories.indexOf("calendar") > -1;

return (
<>
<div className="px-4 pb-3 pt-8 sm:pt-2 md:px-8 lg:px-0 lg:pt-0">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented this code using backPath and heading props supported by Shell.

@hariombalhara hariombalhara requested review from a team and PeerRich September 24, 2022 06:51
@hariombalhara hariombalhara marked this pull request as ready for review September 24, 2022 06:51
@hariombalhara hariombalhara marked this pull request as draft September 24, 2022 07:11
Copy link
Member

@PeerRich PeerRich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job

@PeerRich PeerRich merged commit ab417ac into main Sep 24, 2022
@PeerRich PeerRich deleted the hide-mobile-nav-on-subpages branch September 24, 2022 11:55
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core area: core, team members only
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Mobile: hide bottom nav on any sub page
2 participants