search

Shopify / shopify-app-bridge

https://shopify.dev/docs/api/app-bridge
86 stars 9 forks source link

Overlap between app bridge and Polaris page layout? #234

Closed da-amo closed 1 year ago

da-amo commented 1 year ago

Hello,

We are working to implement Built for Shopify guidelines within our app layout. We're using the Polaris page layout, which includes an element for page title and allows for primary and secondary actions.

https://polaris.shopify.com/components/layout-and-structure/page

However, the title-bar component of the app bridge seems duplicative. This also sets a page title, and allows for primary and secondary actions. We'll have page title in two places, with action buttons available in each.

https://shopify.dev/docs/api/app-bridge-library/reference/title-bar

The title-bar component seems like it is required if we want to implement the ui-nav-menu element, to display our navigation in the Shopify side bar.

Can you please clarify if the title-bar component is required, or if there is a way to implement the app bridge ui-nav-menu element without it in a way that satisfies Built for Shopify guidelines?

Shopify support suggested I raise the issue in this repository

Thank you

belalsj commented 1 year ago

Hello @da-amo,

We appreciate your attention to details to tailor your app to be Built for Shopify.

We recommend that you only use the title-bar component to define your title and primary/secondary actions and not the Page component. This way you will avoid rendering the title and action buttons twice.

da-amo commented 1 year ago

Thank you @belalsj

We're having some trouble with this, because the page title component is already fully integrated into our app and working quite well:

image

We feel like the title-bar component has more limited functionality, and would prefer not to switch for this specific use case. We also don't want to duplicate elements (title and main actions) on our page, because we feel like this would be a subpar customer experience.

Our primary goal is to add the ui-nav-menu element: https://shopify.dev/docs/api/app-bridge-library/reference/navigation-menu

It seems like this can only be done in a way that also enables the app bridge title-bar. Is there any other way we can enable the Navigation Menu in order to adhere to Built for Shopify guidelines in a way that also works with the Page title bar?

Thank you

belalsj commented 1 year ago

Hi da-amo,

There is another way to add Navigation Items for your app. On the App's page on your Partners account:

  1. Navigate to App Setup.
  2. In the Embedded App section > click Manage
  3. In the Navigation section > click Configure

Give this a go and let us know if that covers your use case.

da-amo commented 1 year ago

Thank you @belalsj. I did come across this previously, but it wasn't clear to me if this would satisfy the Built for Shopify requirement to use the embedded navigation.

The Shopify design guidelines specifically mention the App Bridge NavigationMenu component as a "must do":

image

I think the Embedded App Navigation configuration is a viable approach for us, but I want to make sure that it won't cause a rejection for Built for Shopify if we go that route.

Thanks for the continued help here!

MitchLillie commented 1 year ago

@da-amo

Apologies for the confusion. You can continue to use Page rather than ui-title-bar if it suits your use case. It will not affect your BFS status.

It seems like this can only be done in a way that also enables the app bridge title-bar.

You can also use ui-nav-menu to set your navigation items. Doing so should not enable the title-bar. Can you share exactly what you're seeing so we can further debug this issue?

da-amo commented 1 year ago

Thanks @MitchLillie. You've helped to address my issue, we'll leave the Page title as is and will enable the nav menu via the approach that @belalsj shared:

  1. Navigate to App Setup.
  2. In the Embedded App section > click Manage
  3. In the Navigation section > click Configure

Thanks again!

MitchLillie commented 1 year ago

@da-amo Great! Glad to hear you've figured it out.

For our own elucidation, can you please share what the problem was with ui-nav-menu? We've tried to reproduce your issue by creating an app with ui-nav-menu alongside Page and it seemed to work and look fine. But your comments suggested there was some undesired behavior you were experiencing.

da-amo commented 1 year ago

@MitchLillie happy to help, hopefully it's just user error on our part and not an actual issue. I asked my developers for some detail and this is what they shared:

The only way we had been able to get the Navigation Menu to appear on the sidebar was by adding the App Bridge. The moment we added the App Bridge, the Title Bar would start showing up as well, taking a considerable amount of space and causing the page title to appear twice, once outside the iframe, and again within the iframe.

We were not sure what to do about that. We thought maybe we were expected to move the functionality in the title area of our Page component to the Title Bar.

A screenshot of what we saw:

image

MitchLillie commented 1 year ago

@da-amo Thanks for the context. This might be related to a bug we're currently working on. If your app sets document.title, app-bridge interprets that the same as if you were using the title bar component.

In any event, glad to hear you have things working. Let us know if you need anything else.