Shopify / shopify-app-bridge

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

Contextual Save Bar not preventing navigation when using NextJS app router in Shopify app #313

Open noelgoodday opened 3 months ago

noelgoodday commented 3 months ago

Describe the bug

The contextual save bar is not preventing navigation when using NextJS app router in Shopify app.

To Reproduce

Navigating via router:

  1. Launch a NextJS app in Shopify with two routes (/a and /b)
  2. Navigate to route A: router.push('/b')
  3. Enable contextual save bar to prevent navigation
  4. Navigate to route B: router.push('/b')
  5. The contextual save bar is showing the expected dialog and the URL doesn't change but the NextJS router continues to route to B

Navigating via browser:

  1. Launch a NextJS app in Shopify with two routes (/a and /b)
  2. Navigate to route A: router.push('/b')
  3. Navigate to route B: router.push('/b')
  4. Enable contextual save bar to prevent navigation
  5. Hit browser back button
  6. The contextual save bar is showing the expected dialog and the URL doesn't change but the NextJS router continues to route to B

Trying to navigate to any Shopify link using the Shopify menu on the left is prevented as expected.

Expected behaviour

The contextual save bar should prevent navigation and the NextJS router shouldn't be aware of that navigation.

Packages and versions

Using latest version of App Bridge: <script src="https://cdn.shopify.com/shopifycloud/app-bridge.js" />

Platform

DamirAid commented 3 months ago

the same issue with history from react-router-dom

DamirAid commented 3 months ago

@henrytao-me Hi, are there any updates? or maybe there is a workaround? this is the last bug that separates our application from the deploy of the new app-bridge, Thanks!

henrytao-me commented 3 months ago

Not yet @DamirAid. This is on my list. I am trying to get to this as soon as I can.

henrytao-me commented 3 months ago

The temporary workaround is using ui-save-bar or SaveBar react component, grab the saveBar instance and call .showing, then prevent navigation manually.

Note that: .showing is undocumented. We will either keep it as-is if we have a proper solution for this issue OR document it in the public docs if there is no viable solution that works for all cases.

var anchorTag = document.getElementById('....something...');
var uiSaveBar = document.getElementById('my-save-bar');

anchorTag.addEventListener('click', (event) => {
  if (uiSaveBar.showing) {
    event.preventDefault();
  }
})
henrytao-me commented 3 months ago

@DamirAid try both

    if (saveBarElement?.showing) {
      event.preventDefault();
      event.stopPropagation();
    }
amoser67 commented 3 months ago

I am using Remix and noticing a similar issue, but only when the browser back button is clicked. If a user clicks the navigation menu in the sidebar, refreshes the page, or types in a new URL and attempts to navigate to it, the contextual save bar blocks them appropriately. However, if the back button is clicked in the browser, I end up with the same bug described above, where the save bar shows the confirmation modal but doesn't prevent navigation.

amoser67 commented 3 months ago

@henrytao-me Can you provide any insight into what the timeline for this fix might look like? Trying to determine if it's worth delaying app review/launch, or if we should just remove it for now and look to add it back in once its fixed. (edited)

DamirAid commented 3 months ago

@amoser67 I was able to prevent navigation when clicking on the Polaris components, now I show my own leave confirmation modal, but I still couldn’t prevent navigation when clicking on the browser back button, I think it’s worth waiting for a fix or it would be nice to be able to programmatically disable the leave comfirmation feature now

DamirAid commented 3 months ago

@amoser67 you need to tag @henrytao-me about timeline for this fix

amoser67 commented 3 months ago

Oops, thanks. Agreed, we either need a way to disable the leave confirmation and handle ourselves, or we need a fix for the back button.

henrytao-me commented 3 months ago

@amoser67 I planned to work on this next week.

axis80 commented 3 months ago

I am running into this issue when clicking on the backAction defined on the Polaris "Page" component:

image

Using react-router-dom 6.15.0.

darrynten commented 3 months ago

Same issue with a Turbo app.

It also affects browser forward button.

DamirAid commented 3 months ago

@henrytao-me hello, have you any updates?

henrytao-me commented 3 months ago

A fix for this is coming...

darrynten commented 2 months ago

Has .showing stopped working?

works

henrytao-me commented 2 months ago

Hi all,

We shipped a support for this in the latest version. You can await for leaveConfirmation like below before the actual navigation. This promise will resolve when there is no saveBar or after merchant confirms to leave.

Public docs will be updated shortly.

button.addEventListener('click', async () => {
  await shopify.saveBar.leaveConfirmation();
  // Do your routing here
});

@darrynten @DamirAid

DamirAid commented 2 months ago

@henrytao-me Thanks for update! what about same bug with browser back buttton?

darrynten commented 2 months ago

Thank you for this @henrytao-me but now how do we apply this new feature to cancelling the browser back and forward button navigation?

henrytao-me commented 2 months ago

Has .showing stopped working?

It still works. Maybe you are checking uiSaveBar.showing but the visible SaveBar is currently controlled by <form data-save-bar>

henrytao-me commented 2 months ago

@DamirAid @darrynten do you have specific examples to reproduce the back button? LeaveConfirmation for back and forward buttons are controlled by Admin.

DamirAid commented 2 months ago

@henrytao-me when the savebar enabled and visibled, if you click on the browser back button, navigation is not prevented, while the leave confirmation modal is shown on the next page, I’ll try to record a video a little later while, if you navigate through the app bridge navmenu, then everything works as it should

henrytao-me commented 2 months ago

Can I also have the appId and your tech stack too? 🙇

DamirAid commented 2 months ago

@henrytao-me 5a5d1537ce273a617ca4ae5905756663 - shopify api key react 18, react-router-dom - stack

also sent a video with this bug on your email, thanks

darrynten commented 2 months ago

@henrytao-me

appid: it's a dev app so it's only online when i'm online

tech stack: turbo and stimulus

What's notable in this case is that the save bar is inside a "fallback" max modal initiated by the v3 Fullscreen action (this is the only way I can achieve an instantly opening max modal without any interstitial loading spinner for the static contents as described in #314)

However, even when manually showing the save bar outside of the max modal with shopify.saveBar.show('my-save-bar') in the console the forward history navigation is not ignored.

As mentioned by @DamirAid the side menu navigation is correctly blocked.

Also, re .showing this was indeed related to #334

muchisx commented 2 months ago

@henrytao-me the promise doesn't return the result of the user desition, how do i decide if they clicked cancel or leave the page ?

darrynten commented 2 months ago

Updated forward button behaviour

I've been working on a minimal step-by-step reproduction for entering the broken forward-button state.

Reproduction

  1. Load main app page (A)
  2. Click any side menu nav to go to secondary page (B)
  3. Click browser "back" button to go back to page A
  4. In the console do window.shopify.saveBar.show('my-save-bar')
  5. Click browser "forward" button

You will see that page B is loaded in the background, with the confirm modal displayed in the foreground.

This behaviour is incorrect, as the user should remain on page A until they've confirmed they want to navigate forwards to page B.

image

Along with a warning in the console that: A router only supports one blocker at a time

image

This warning shows only after clicking the forward button in step 5

When "Leave page" is clicked

Besides the fact that navigation happens before the confirm dialog is shown the rest works fine:

  1. The modal disappears as expected
  2. The user remains on page B as expected
  3. URL updates to page B URL
  4. The top save bar closes as expected
  5. Side menu navigation continues to work as expected

When "Cancel" is clicked

Things start to fall apart when you cancel the confirm dialog:

  1. The modal disappears as expected
  2. The user remains on page B (should remain on page A)
  3. URL does not change away from page A to page B
  4. Unsaved changes bar remains but "save" and "discard" buttons no longer trigger events (there is no ui-save-bar component in the markup for page B)
  5. Side menu navigation stops working completely and the user is stuck on page B (they get a confirm prompt on each attempted navigation)
  6. If they eventually select "Leave page" instead of "Cancel" during this process the top bar goes away but all side menu navigation stops working completely leaving user forever stuck on page B
ryanrphillips commented 2 months ago

We're using Tanstack Router and the following seems to resolve the issues we were having:

router.subscribe('onBeforeLoad', async () => {
  /* @ts-ignore */
  await shopify.saveBar.leaveConfirmation()
})

The only issue we have now:

Page A has a form/save bar. Page B does not have a form/save bar, but has an app-bridge modal.

Navigating from Page A with a dirty form (save bar showing) to Page B results in the confirmation modal appearing. I click "Leave page". I land on Page B and the save bar appears again. None of the buttons in the save bar work.

If I remove the app-bridge modal from Page B, the navigation works fine and the save bar doesn't appear on that page.

henrytao-me commented 2 months ago

Thanks everyone for feedback. I will look into all of them 🙇

jamesdirosa commented 2 months ago

@DamirAid @darrynten do you have specific examples to reproduce the back button? LeaveConfirmation for back and forward buttons are controlled by Admin.

@henrytao-me, I am not sure if this is specific to my issue (happy to chat), but I noticed a console error of A router only supports one blocker at a time when this error occurs. I am wondering if another blocker is registering, preventing the admin from properly handling this.

maruffahmed commented 1 month ago

Recently, we upgraded to app-bridge 4 and encountered this issue. Has there been any progress on resolving it? Our tech stack is React with react-router.

maruffahmed commented 1 month ago

Hi @henrytao-me

Just like the discardConfirmation dialog is optional can we also make the Navigation blocker optional so we can implement our own navigation blocker using something similar like useBlocker from react-router?

raquelbreternitz commented 1 month ago

I'd love to add to the general discussion here and exhort you all to work on this bug! 🙏🏼

As we're using Next, our embedded app can't use the contextual save bar. As a stand-in, we're currently setting aside the primary Page Action on every page for "save," which as you can imagine is not the best user experience and lacks the functionality of indicating when there are unsaved changes.

Rather than rebuild our own version from scratch or continue to add hacky solutions, I would love to be able to simply use the correct CSB, and become eligible to pursue Built for Shopify on top of that. We are not able to meet BFS with AppBridge, and we need to be using Polaris except that it's constrained to the iFrame, which doesn't look great, and the Polaris component is considered Deprecated and encourages us to use the AppBridge component which, of course, does not work.

Thank you!

elvonkh commented 1 month ago

Any news on this? Shopify is forcing us to update to V4, however we cannot upgrade it unless all bugs are fixed.

Is is possible to use ContextualSaveBar from v3 separately?

@raquelbreternitz I am experiencing similar problem here. Did you find any solution for this matter?

bithaolee commented 3 weeks ago

any update?

maruffahmed commented 1 week ago

The latest behavior of the App Bridge 4 contextual Save Bar (Centralized new Save Bar UI):

It is now successfully preventing navigation. However, it has failed to navigate to other routes of the app in the following scenario:

When attempting to edit a form and trying to press the browser back button with unsaved values, the navigation will be blocked with the contextual Save Bar shaking effect. Press the discard button to reset the form and try to navigate to other app pages using the app's sidebar menu (Navmenu). It seems like the app navigation is no longer working.

I guess it's an App Bridge issue.