Shopify / shopify-app-bridge

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

Loading/Disabled state not working for SaveBar buttons in max Modals #375

Open Zetxus opened 3 weeks ago

Zetxus commented 3 weeks ago

Describe the bug

Max modals seem to be able to have their own SaveBars. None of the other sizes work with them as it seems like the position to embed it gets shifted to the underlying page instead.

However, when you try to use a SaveBar in a max Modal, even though the SaveBar gets attached correctly to the header of the modal, none of the states are properly displayed - neither loading not disabled are properly reflected.

To Reproduce

Steps to reproduce the behaviour:

  1. Create an app from the remix template
  2. Replace app._index.tsx content with the following short demo snippet

    import type { LoaderFunctionArgs } from "@remix-run/node";
    import { Modal, SaveBar } from "@shopify/app-bridge-react";
    import {
    Page
    } from "@shopify/polaris";
    import { authenticate } from "../shopify.server";
    
    export const loader = async ({ request }: LoaderFunctionArgs) => {
    await authenticate.admin(request);
    
    return null;
    };
    
    export default function Index() {
    return (
      <Page>
        <Modal open variant="max">
          <SaveBar open>
            <button variant="primary" disabled></button>
            <button loading=""></button>
          </SaveBar>
        </Modal>
      </Page>
    );
    }
  3. See that neither of the buttons reflect their state (loading/disabled) image

Expected behaviour

I don't know what the idea was, but the way I see it we should be able to use the SaveBar in the modals. Think of all the apps that have big WYSIWYG editors and are now trying to migrate to v4.

Contextual information

Packages and versions

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

Platform

Additional context

N/A

darrynten commented 1 week ago

Maybe the comments in https://github.com/Shopify/shopify-app-bridge/issues/334 will be useful for your case

Zetxus commented 1 week ago

@darrynten I'm actually just using SaveBar by itself so it shouldn't affect the outcome.

It looks like the SaveBar attaches itself to the new document (from the max modal iframe) but the actions do not and are probably attached to the document below it (the app one). Lots of iframe juggling here, I wish they'd just remove them from the modals entirely 😔

Seems like a missed edge case because the max modal should absolutely be able to handle SaveBars as max modals are mostly used for WYSIWYG editors, which most of the time include save/discard actions.

YevhenTarashchyk commented 1 day ago

same problem. Any fix ?