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

What happend to Modal App Bridge #238

Open
aimproxy opened this issue Sep 23, 2023 · 32 comments
Open

What happend to Modal App Bridge #238

aimproxy opened this issue Sep 23, 2023 · 32 comments
Labels
bug Something isn't working Polaris overlay bug

Comments

@aimproxy
Copy link

Modal in App Bridge is no longer supported in the most recent version. I feel like modals are an important part of shopify admin and avoid a lot of back and forward page navigation!...

@aimproxy aimproxy added the bug Something isn't working label Sep 23, 2023
@charlesdobson
Copy link
Contributor

It's currently being worked on and will be released soon! I'll update here when it's out

@aimproxy
Copy link
Author

aimproxy commented Sep 23, 2023

Any workaround I can get it done right now? Perhaps old version of App Bridge I can use?

@charlesdobson
Copy link
Contributor

For now, your best bet is to use one of the App Bridge npm packages if you need the modal. https://shopify.dev/docs/api/app-bridge/previous-versions/actions/modal#react

I don't have a date that Modal will be released on the new App Bridge, but it should be within the next month or two! Just wrapping up some final API considerations.

@kirillplatonov
Copy link

Looking forward to it. Missing Modal is deal breaker for App Bridge 3 > 4 upgrade at the moment.

@resistorsoftware
Copy link

Looking forward to it. Missing Modal is deal breaker for App Bridge 3 > 4 upgrade at the moment.

ARGGGGG... if I had know this library was unusable like this, I would never have adopted it for my latest updates. I feel ripped off and have wasted hours. Grrrrr...

@darrynten
Copy link

Button is also missing

4.x seems like a big L

@charlesdobson
Copy link
Contributor

@darrynten The App Bridge from npm Button was only used to create the contract for Modal and TitleBar buttons. You can just use a regular HTML button now when you use things like the Title Bar. Did you have a usage for it outside of that?

@darrynten
Copy link

We currently use buttons:

  • to make ButtonGroups (another missing 4.x Action)
  • entering and exiting Fullscreen (another missing 4.x Action)
  • on the TitleBar
  • on Modals

@charlesdobson
Copy link
Contributor

We added Modals with custom DOM content! Check it out: https://shopify.dev/docs/api/app-bridge-library/reference/modal

@mgill404
Copy link

I am having issues using ui-modal in a Remix app.

My react components seem to not call their interaction callbacks with this modal. Buttons don't call onClick for example. Any advice on how to address this issue?

for a brief example, here is a Remix route

import { Button } from "@shopify/polaris";
import { useEffect } from "react";

export default function Modal() {
  function handleOnClick(e) {
    console.log("received click event"); // does not log
  }

  useEffect(() => {
    document.getElementById("test-modal").show();
  }, []);

  return (
    <ui-modal id="test-modal" variant="max">
      <Button onClick={handleOnClick}>A Button</Button>
    </ui-modal>
  );
}

@daviareias
Copy link

daviareias commented Jan 14, 2024

I am having issues using ui-modal in a Remix app.

My react components seem to not call their interaction callbacks with this modal. Buttons don't call onClick for example. Any advice on how to address this issue?

This is because they are using webcomponents, but for some reason Polaris uses React, the only way that I managed to fix this is this to wrap the "ui-modal" around a ref, can probably also give a ref to ui-modal, but typescript will indicate an error:

export default function Modal() {
    const ref = useRef < HTMLDivElement > null;
    const handleOnClick = () => {
        console.log("received click event");
    };

    return (
        <div ref={ref}>
            <ui-modal id="test-modal" variant="max">
                <button onClick={handleOnClick}>A Button</button>
            </ui-modal>
        </div>
    );
}

@coderlinbb
Copy link

我在 Remix 应用程序中使用 ui-modal 时遇到问题。
我的反应组件似乎没有用这个模式调用它们的交互回调。例如,按钮不会调用 onClick。关于如何解决这个问题有什么建议吗?

这是因为他们使用的是 web 组件,但由于某种原因 Polaris 使用 React,我设法解决这个问题的唯一方法是将“ui-modal”包裹在 ref 周围,也可以给 ui-modal 提供引用,但打字稿会提示错误:

export default function Modal() {
    const ref = useRef < HTMLDivElement > null;
    const handleOnClick = () => {
        console.log("received click event");
    };

    return (
        <div ref={ref}>
            <ui-modal id="test-modal" variant="max">
                <Button onClick={handleOnClick}>A Button</Button>
            </ui-modal>
        </div>
    );
}

Still can't

@daviareias
Copy link

daviareias commented Jan 17, 2024

我在 Remix 应用程序中使用 ui-modal 时遇到问题。
我的反应组件似乎没有用这个模式调用它们的交互回调。例如,按钮不会调用 onClick。关于如何解决这个问题有什么建议吗?

这是因为他们使用的是 web 组件,但由于某种原因 Polaris 使用 React,我设法解决这个问题的唯一方法是将“ui-modal”包裹在 ref 周围,也可以给 ui-modal 提供引用,但打字稿会提示错误:

export default function Modal() {
    const ref = useRef < HTMLDivElement > null;
    const handleOnClick = () => {
        console.log("received click event");
    };

    return (
        <div ref={ref}>
            <ui-modal id="test-modal" variant="max">
                <Button onClick={handleOnClick}>A Button</Button>
            </ui-modal>
        </div>
    );
}

Still can't

I tried it here and it won't work with Polaris "Button" you have use the HTML native "button" instead.

I don't know why they want us use to use the modal API when react has it's own logic that doesn't integrate well with other technologies.

@coderlinbb
Copy link

I used the HTML native "button" instead,but still can't.Help!!!

@maxfrigge
Copy link

The "native" components are great, until they are not.
When you hit a road block, you can't really do anything about it because they are encapsulated from your code.

In my experience, just switching to a legacy component (e.g. Polaris Modal) when things don't work your way is a very productive solution. You can always revisit the other approach at a later point in time.

@muchisx
Copy link

muchisx commented Jan 31, 2024

I'm using the Remix template for development of my app but going out of the React architecture with this web-component is rather inconvenient.

I can't even use interactive Polaris components inside that modal such as buttons with click callbacks because they don't work anymore.

Anyone here using the Remix template, what did you do to remedy this? Use the previous version of app brigde react? use deprecated polaris modals? make your own modals?

would love to know and get inspired by your ways so I can find the best solution to use Polaris inside my modals!

@Michael-Gibbons
Copy link

I also cannot get this modal to work. The Polaris modal component is deprecated, callbacks not working for me. And the replacement isn't working. So am I just SOL?

@kaczors
Copy link

kaczors commented Feb 13, 2024

@charlesdobson hi :)
Is there somewhere working example - the best using standard remix/polaris template?
For example how to call

shopify.modal.show('modal-id')

or how to not get VS error

Property 'ui-modal' does not exist on type 'JSX.IntrinsicElements'

@muchisx
Copy link

muchisx commented Feb 13, 2024

@charlesdobson hi :) Is there somewhere working example - the best using standard remix/polaris template? For example how to call

shopify.modal.show('modal-id')

or how to not get VS error

Property 'ui-modal' does not exist on type 'JSX.IntrinsicElements'

For this, just update the shopify packages in your package.json.


But to continue the actual problem, can we make somehow this component load the react needed for it to work within its iFrame if used within a React/Remix context ?

@tylerhellner
Copy link

The modal component itself works well and seems to be using native Browser APIs under the hood (good), but I am flabbergasted why they would build a Modal with its own internal API for state. This completely breaks how React conceptualizes state, and it's incompatible with other Polaris components. What was the reason behind the change?

@Michael-Gibbons
Copy link

@tylerhellner to "increase friction" to prevent "incorrect usage" Shopify/polaris#11460

@ivorpad
Copy link

ivorpad commented Feb 15, 2024

I'm astounded by the choice to eliminate Modal support in the newest release. Modals play a key role in improving user experience and simplifying navigation. It's disheartening that this functionality has been phased out without offering a clear substitute.

@muchisx
Copy link

muchisx commented Feb 20, 2024

I have then now resorted to using the Modal from @shopify/app-bridge-react, which doesn't work perfectly as it works with routes, not with children and I get this strange error while working with the remix template.
Shopify/shopify-app-template-remix#541

But so far is the only way I can have a modal and stay within the React system.

@CodeChd
Copy link

CodeChd commented Feb 22, 2024

Does anyone have a workaround for this yet? I'm facing the issue that callbacks are not working for me.

@charlesdobson
Copy link
Contributor

We've just released App Bridge React v4 which includes a React wrapper for the ui-modal element! This sets up the React portal for you, so you can just do

<Modal>
  <MyComponent />
  <AnotherComponent />
</Modal>

https://shopify.dev/docs/api/app-bridge-library/react-components/modal


See the migration guide for more info

@handhikadj
Copy link

handhikadj commented Mar 5, 2024

how do I prevent ui-modal to be closed when the backdrop or the close (x) button is clicked?
can't see such prop in the docs

@dungfv
Copy link

dungfv commented Mar 6, 2024

I noticed that many Polaris components cannot work in the new modal:

  • Popover
  • Tooltip
  • Drop zone

Additionally, drag & drop behaviors are also blocked in the new app bridge modal.
This breaks my current application.

@darrynten
Copy link

darrynten commented Mar 6, 2024

@handhikadj see #297 for how to listen to the event, maybe you can cancel the event or just call the open event again in the close event

@hmpws
Copy link

hmpws commented Mar 8, 2024

useEffect(() => { setModalOpen(true); }, []);

How do I have the modal open on page load (conditionally to display a message)? App Bridge keeps giving me error. Using remix-react.

image

@agustinlaurito
Copy link

I noticed that many Polaris components cannot work in the new modal:

  • Popover
  • Tooltip
  • Drop zone

Additionally, drag & drop behaviors are also blocked in the new app bridge modal. This breaks my current application.

Any update regarding this issue? I'm attempting to utilize the datepicker within the modal's popover, but it's not functioning correctly. Instead, the popover appears on the page behind the modal.

@shatishdesai202
Copy link

#331
this one is also an issue with the new app-bridge version (v 4.1.2)

@noelgoodday
Copy link

I noticed that many Polaris components cannot work in the new modal:

  • Popover
  • Tooltip
  • Drop zone

Additionally, drag & drop behaviors are also blocked in the new app bridge modal. This breaks my current application.

Any update regarding this issue? I'm attempting to utilize the datepicker within the modal's popover, but it's not functioning correctly. Instead, the popover appears on the page behind the modal.

We're experiencing the same issues with drag & drop in the new AppBridge modal. I opened a dedicated issue here: #364

vividviolet added a commit to Shopify/polaris that referenced this issue Aug 1, 2024
### WHY are these changes introduced?
Fix issues related to overlays not working correct in App Bridge modals
as outlined in this
[doc](https://docs.google.com/document/d/1eocCl07pk8xt7rvUaGDWZzfJxxeJ2yRi1ul0x0ezkDM/edit).

These specific issues will be fixed
- Shopify/shopify-app-bridge#316
-
Shopify/shopify-app-bridge#264 (comment)
-
Shopify/shopify-app-bridge#238 (comment)
- Shopify/shopify-app-bridge#301

### WHAT is this pull request doing?

Currently Overlays are broken when rendered using React portals. This PR
fixes the issue by doing the following:
- ensuring all references to `document` are updated to be
`node.ownerDocument` instead so the positioning can be calculated
correctly inside of iframes
- ensuring wherever we do `instanceOf HTMLElement` we instead just check
to see if the node is present or try to call the related methods
(`getBoundingClientRect`) with a fallback as the `instanceof` check will
fail when using React portals for iframe content since iframes have
their own globals

This PR only touches the related utils for Overlays but there are other
references to `document` and `instanceOf HTMLElement` we should also
update.

Expected usage with App Bridge v4, note that you have to wrap the
modal's content with the AppProvider from Polaris so that the portals
are created in the correct place.

For example, here's a modal with tooltips inside:

```jsx
import {AppProvider} from '@shopify/polaris';
import {Modal} from '@shopify/app-bridge-react';

function App() {
  return (
    <Modal>
      <AppProvider i18n={{}}>
        <div style={{padding: '75px 0'}}>
          <Tooltip content="This order has shipping labels.">
            <Text fontWeight="bold" as="span">
              Order #1001
            </Text>
          </Tooltip>
          <ButtonGroup variant="segmented" fullWidth>
            <Tooltip content="Bold" dismissOnMouseOut>
              <Button>B</Button>
            </Tooltip>
            <Tooltip content="Italic" dismissOnMouseOut>
              <Button>I</Button>
            </Tooltip>
            <Tooltip content="Underline" dismissOnMouseOut>
              <Button>U</Button>
            </Tooltip>
            <Tooltip content="Strikethrough" dismissOnMouseOut>
              <Button>S</Button>
            </Tooltip>
          </ButtonGroup>
        </div>
      </AppProvider>
    </Modal>
  );
}

```

**Before**


https://github.com/user-attachments/assets/f7b7a7df-f25f-422c-a5bc-d1a8d7f12167


**After**

https://github.com/user-attachments/assets/6c1bf97a-e8f4-4715-9960-9f5709222f75


https://github.com/user-attachments/assets/ebc4755e-cc3b-413a-aaf5-9f39cc7b7ad0


### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

1. Open the App Bridge Playground:
https://admin.shopify.com/store/trish-dev/apps/app-bridge-next-playground-4/react/index.html
2. Open all the custom content modals and do a smoke test to verify
overlay components are rendered on top with correct sizing and click
events are working

### 🎩 checklist

- [X] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [X] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [X] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
~- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)~
~- [ ] Updated the component's `README.md` with documentation changes~
~- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Polaris overlay bug
Projects
None yet
Development

No branches or pull requests