Fix closeOnClickOutside for the Popover component

[magicmq]

Summary

Beginning with Mantine version 7.13.4, the behavior of the onClose prop for the Popover component was changed in relation to other bug fixes within the project. Previously, the onClose prop was called when a user action triggered the dropdown (which meant onClose could be used to control the state of the popover). However, onClose is now called when the Popover actually closes, not when a trigger occurs. Therefore, onClose is no longer a viable method for controlling the Popover's state.

The Mantine developers recommend using the onChange prop instead to control the Popover's state (mantinedev/mantine#7019). As such, this PR updates the Popover component to utilize the onChange method for closing the Popover rather than onClose.

This PR should also fix closeOnEscape, as this too previously relied upon the onClose prop to function correctly.

Closes #420.

Changes

• The Popover component now utilizes the onChange prop to control its state.
• Added a description to the Popover component (work towards improve Python docstrings #421).
• Updated the popover test to include testing for closure on clicking outside.
• Updated the CHANGELOG.md.

Example

from dash import Dash, html, _dash_renderer
import dash_mantine_components as dmc

_dash_renderer._set_react_version("18.2.0")

app = Dash()

app.layout = dmc.MantineProvider(dmc.Box(
    [
        dmc.Popover(
            [
                dmc.PopoverTarget(
                    dmc.Button('Toggle Popover')
                ),
                dmc.PopoverDropdown(
                    dmc.Stack(
                        [
                            dmc.Title('A Popover', size='lg', order=4, c='black'),
                            dmc.Text('Popover Text')
                        ],
                        gap='xs'
                    )
                )
            ]
        )
    ],
    m=10
))

app.run_server(debug=True)

The fix is tested working with the above example, but I have not run other testing with pytest so I am not sure if the change is 100% passing. Let me know if you have any feedback or if you would like me to run additional testing.

[github-actions]

Generated link: snehilvj/dash-mantine-components-423

[AnnMarieW]

Hi @magicmq
Nice work and thank for submitting your first PR! 🏅

I'll take a closer look in a bit, but in the meantime, here is the sample app hosted on PyCafe.

Note that the app is using the build from this point in the PR. It's editable so you can try different scenarios too.

[AnnMarieW]

Again, nice job on the PR. Cool that it can be fixed in one line :-)

Just a couple tips:

• It's a good practice to create a branch in your cloned dmc repository for making changes, instead of using the master branch. No need to update anything for this PR—just a note for future contributions.

• Great job describing the issue and your solution! If you include:

closes #420  

in your PR description, the associated issue will automatically close when the PR is merged, and it will indicate there's an active PR linked to that issue.

---

Additions

• Feel free to add any missing steps to the Contributing Guide. We'd like to make it as easy as possible for new contributors.

• To chip away at improve Python docstrings #421, please add a one sentence description of the Popover component. Can you change line 8 so the description reads something like:

/** Use the Popover  to display additional content in a dropdown element triggered by user interaction with a target element. */

• Need to add a Changlog entry. starting on the second line, add:

# Unreleased

### Fixed
- Enabled closing popup when clicking outside #423 by @magicmq

---

Tests

Have you tried following the instructions in the Contributor Guide on getting test to run locally? It can be a little tricky, but it's pretty cool once you get the hang of it.

To update the current Popover test, we can extend it to check that the Popover closes when clicking outside. We can include an extra element in the test and using Selenium, locate this element, click on it, and then confirm that the Popover is closed.

Here is the minimal app on PyCafe

Here it is turned into a test:

def test_001po_popover(dash_duo):
    app = Dash(__name__)

    app.layout = dmc.MantineProvider(
        html.Div(
            [
                html.Div(id="click-outside", children="click outside", style={"backgroundColor": "grey"}),
                html.Div(id="output"),
                dmc.Popover(
                    [
                        dmc.PopoverTarget(dmc.Button("Toggle Popover", id="btn")),
                        dmc.PopoverDropdown(dmc.Text("This popover is opened")),
                    ],
                    id="popover",
                    opened=False,
                ),
            ]
        )
    )

    @app.callback(Output("output", "children"), Input("popover", "opened"))
    def update_output(opened):
        return str(opened)

    dash_duo.start_server(app)

    # Wait for the app to load
    dash_duo.wait_for_text_to_equal("#output", "False")

    popover_btn = dash_duo.find_element("#btn")
    popover_btn.click()

    # Verify the dropdown is open
    dash_duo.wait_for_text_to_equal("#output", "True")

    # verify the dropdown closes when clicking outside
    dash_duo.find_element("#click-outside").click()
    dash_duo.wait_for_text_to_equal("#output", "False")

    assert dash_duo.get_logs() == []

[magicmq]

Hi @AnnMarieW,

My apologies for not creating a new branch in my fork prior to submitting the PR (still learning!). Since this was a relatively simple fix, I can start over and commit to a branch this time and recreate the PR, along with commits for the other additions you requested above. Would you like me to do that?

I will look into testing locally (as well as modifying the test for the Popover component) now. I didn't get that far yesterday.

[AnnMarieW]

Hey @magicmq it's not critical to create a new branch. It's ok to carry on from here as is. It's nice to keep all the comments with the PR.

[magicmq]

I added testing for closure on click outside to the Popover test, and it passes! All other tests pass as well with the exception of the carousel test, which I am assuming is unrelated to this change.

I also committed the other changes you requested. Let me know if there's anything else that's needed!

[AnnMarieW]

This looks great! 💃
I'll just see if Alex has any comments before merging, but it looks good to me 🙂

[alexcjohnson]

💃 looks great, very nice additions to the test!

[AnnMarieW]

Congrats on you first DMC PR @magicmq 🥳

[magicmq]

Thank you both!