`ui-modal` is showing a loading spinner for static content

darrynten commented 3 months ago

Describe the bug

The ui-modal is showing a loading spinner for static contents (i.e. when not using the src attribute)

To Reproduce

Steps to reproduce the behaviour:

Add a ui-modal component to an app with just a single <p> tag and a ui-title-bar component
Call .show() on the modal
See a loading spinner before the contents of the <p> tag is rendered

<ui-modal id="example-modal">
  <p>This content should be rendered immediately, without any loading spinners.</p>
  <ui-title-bar title="Example Static Modal">
    <button variant="primary" onclick="console.log('primary')">Action A</button>
    <button onclick="console.log('secondary')">Action B</button>
  </ui-title-bar>
</ui-modal>

document.getElementById('example-modal').show()

Expected behaviour

A loading spinner should only ever be shown if the src attribute is used when loading remote content as static content is already available when the show() method is called.

Static content should be rendered immediately like it is with the Modal action from v3.

Contextual information

Packages and versions

List the relevant packages you’re using, and their versions. For example:

@shopify/app-bridge @ cdn

Platform

OS: All
OS Version: All
App: All

Additional context

Adding a variant to the modal does not make a difference

jzazove commented 3 months ago

We are looking into this

hareshstilyoapps commented 2 months ago

Hi @jzazove

I am also facing the same problem when loading some path like /my/new/page. My app is in react from when I am opening the modal with /my/new/page route which is also another react app but I have configured it with nginx so that it work with same domain. If I open /my/new/page direct in browser then it works fine, but when I open it in modal, it keeps showing the spinner. However, I can confirm that all the resources and api calls went successful with expected data.

GoodJavaJobs commented 2 months ago

Hi @jzazove , Facing the same issue as well. My modal basically full of static content but I still see a spinner for for the first 5 seconds.

hareshstilyoapps commented 2 months ago

@GoodJavaJobs I found the solution. We need to include the following in the modal page as well. That will stop the loading on when page gets loaded in modal.

https://shopify.dev/docs/api/app-bridge-library/react-components/modal#using-a-src-url-to-load-content-communicating-between-the-modal-and-the-parent-window

<meta name="shopify-api-key" content="%SHOPIFY_API_KEY%" />
 <script src="https://cdn.shopify.com/shopifycloud/app-bridge.js"></script>

darrynten commented 2 months ago

We need to include the following in the modal page as well

@hareshstilyoapps that applies when using the src attribute.

This issue is about static content already rendered in the modal.

darrynten commented 2 months ago

This issue seems to be getting worse.

Now, when showing the ui-modal with static content the entire "parent" page (current browser url) is re-requested at the time of opening the modal, and a spinner shows for a long time until the exact same document that initiated the call has finished downloading again.

The modal is also now losing styles while waiting for the parent page to finish sending its stylesheet again, showing default browser styles for elements in the modal until the stylesheet has finished downloading.

henrytao-me commented 2 months ago

@darrynten We released a fix for modal custom content, including:

No longer download full content of main document
Modal should wait for styles to be loaded before showing content

Can you let us know how it goes? Thanks.

Note that: we will preload the modal in a different release.

darrynten commented 2 months ago

@henrytao-me In terms of addressing the new issues discovered yesterday, yes, both of those are resolved now.

We look forward to the preloading functionality.

fabregas4 commented 1 month ago

The modal provides such a poor user experience. You have to wait a few seconds for the modal to open basic, already available content, when no src attribute is passed. Not sure how this isn't getting a higher priority?

nullndr commented 1 month ago

Any news on this?

fabregas4 commented 1 month ago

Noticed another issue with this, kind of related.

Font-sizes within the modal are huge when using Polaris components. That's because it's using the same breakpoints as the embedded app page, and the modal is small.

It is effectively treating the modal iframe as mobile device size, and applying the large text that Polaris does for mobile, which doesn't match the font-size of the content behind the modal at all, when on desktop.

Not easy to override either as Polaris components use rem sizing which refer back to the size at the HTML document level.

nullndr commented 1 month ago

@fabregas4 the mobile-styles issue is already tracked https://github.com/Shopify/shopify-app-bridge/issues/275

darrynten commented 1 month ago

I've started getting the following message when opening modals that contain only static content:

The resource <URL> was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate `as` value and it is preloaded intentionally.

and

app-bridge.js:1 The modal src is missing App Bridge CDN script tag.

The time it takes to open a modal containing a single <p> tag is now also somehow longer than when I first opened this issue.

steveroseik commented 1 month ago

Same issue, any updates?

eliasdiek commented 1 month ago

Same issue, any update on this issue?

steveroseik commented 1 month ago

I believe I found a temporary solution, is to use Modal from polaris and not from app bridge

this link demonstrates how to use banners inside a modal, and it's using it through polaris.

It worked with me and I think it'll work with type of content other than banner

https://polaris.shopify.com/components/feedback-indicators/banner?example=banner-in-a-modal

fabregas4 commented 1 month ago

@steveroseik I think that's the right answer for the moment unfortunately, despite the Polaris Modal being deprecated. I just implemented it and it works really nice. Hopefully the team get the app bridge Modal working a lot better soon. At least they say they are working on it (https://github.com/Shopify/polaris/issues/11460).

iwoodruff commented 1 month ago

I've started getting the following message when opening modals that contain only static content:
The resource <URL> was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate `as` value and it is preloaded intentionally.
and
app-bridge.js:1 The modal src is missing App Bridge CDN script tag.
The time it takes to open a modal containing a single <p> tag is now also somehow longer than when I first opened this issue.

Hey @darrynten – thank you for your attention to modals & for raising the issues to us – we are actively working on your feedback (along with others) & appreciate all the details you've provided

Looking at the above post, there are a few things to address:

I'm not sure the root cause of the preloading warning & need to follow up with my team
for the missing App Bridge CDN script warning, that's intentional since app bridge modals render in a separate iframe from the embedded app home (which allows it to be centered over the Shopify Admin & inherit the latest styles vs centered within the app iframe & pinned to a version of Polaris). since there are two separate (but same origin) window contexts, we want app bridge in both of those contexts so the two can communicate with each other

in terms of the time it takes a modal to render, that's our top priority right now – we're currently working on it (cc @fabregas4)

darrynten commented 2 weeks ago

Any updates on this?

Shopify / shopify-app-bridge