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

ui-modal is showing a loading spinner for static content #314

Closed
darrynten opened this issue Mar 18, 2024 · 22 comments
Closed

ui-modal is showing a loading spinner for static content #314

darrynten opened this issue Mar 18, 2024 · 22 comments
Labels
bug Something isn't working

Comments

@darrynten
Copy link

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:

  1. Add a ui-modal component to an app with just a single <p> tag and a ui-title-bar component
  2. Call .show() on the modal
  3. 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()
image

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

@darrynten darrynten added the bug Something isn't working label Mar 18, 2024
@jzazove
Copy link

jzazove commented Mar 27, 2024

We are looking into this

@hareshstilyoapps
Copy link

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
Copy link

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
Copy link

@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
Copy link
Author

darrynten commented Apr 11, 2024

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Author

@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
Copy link

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
Copy link

nullndr commented May 15, 2024

Any news on this?

@fabregas4
Copy link

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
Copy link

nullndr commented May 22, 2024

@fabregas4 the mobile-styles issue is already tracked #275

@darrynten
Copy link
Author

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
Copy link

Same issue, any updates?

@eliasdiek
Copy link

Same issue, any update on this issue?

@steveroseik
Copy link

steveroseik commented May 29, 2024

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
Copy link

@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 (Shopify/polaris#11460).

@iwoodruff
Copy link

iwoodruff commented May 30, 2024

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
Copy link
Author

Any updates on this?

@anikmoz
Copy link

anikmoz commented Jul 9, 2024

Any update on this ?

@jzazove
Copy link

jzazove commented Jul 10, 2024

There is no longer spinners in the modals.
We also published a page with guidance on when/how to use modals: https://shopify.dev/docs/api/app-bridge/using-modals-in-your-app

@jzazove jzazove closed this as completed Jul 10, 2024
@kirillplatonov
Copy link

@jzazove This is really great. Thank you for these improvements!

@fabregas4
Copy link

fabregas4 commented Jul 18, 2024

@jzazove Thanks! Seems to be working well.

The only thing I'm noticing as lacking vs the Polaris Modal, now that I've switched it over, is the ability to set loading state on the buttons.

On the Polaris Modal, you could do the below and the primary button would display a loading spinner while you're submitting and checking for server-side errors.

primaryAction={{
	content: "Save",
	onAction: handleSaveOrderSubmit,
	loading: saveOrder.state !== 'idle'
}}

The doesn't seem to be an equivalent way to do this using the TitleBar buttons within the app bridge Modal. If there's an easy solution please let me know, otherwise I can raise as a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests