eBay / ebayui-core

Collection of Marko widgets; considered to be the core building blocks for all eBay components, pages & apps
https://ebay.github.io/ebayui-core/
Other
217 stars 99 forks source link

Dialog open attribute can only use once #582

Closed ccinelli closed 5 years ago

ccinelli commented 5 years ago

Bug Report

eBayUI Version: 2.0.5

Description

class {
    onCreate(input) {
        this.state = {
            dialogOpen: false,
        };
    }

    open(opt) {
        this.state.dialogOpen = true;
        //Workaround: this.el.querySelector('.side-dialog').open = true;
    }
};

<lasso-page flags="ebayui-no-bg-icons"/>

<div>
    <div>
        <ebay-button class="open" priority="primary" on-button-click('open')>Open</ebay-button>
    </div>
    <ebay-dialog class='side-dialog' type="right" a11y-close-text="Close Dialog" aria-labelledby="dialog-title" open=state.dialogOpen>
        <p>A dialog</p>
    </ebay-dialog>
</div>

Clicking "Open", it opens the dialog only the first time. I understand why it does not work (the state never change but the behavior is anti-intuitive. The user would like to use dialogOpen as state but they cannot.

DylanPiercey commented 5 years ago

The dialog open attribute is honored by the ebay-dialog if it changes. In this case your issue is that the dialogOpen state is always true after the open method is called. This means that when you call the open method again the next time the it is already true and Marko is not going to detect that the state changed and not cause an update (https://markojs.com/docs/state/#how-updates-work).

You have two options in this case.

  1. You can use this.forceUpdate() in the open method which will force a rerender regardless of if the dialogOpen state has changed.
  2. (Recommended): You can sync your dialogOpen state with the ebay-dialog by listening to its on-dialog-close event.

Here is what option 2 would look like above:

class {
    onCreate(input) {
        this.state = {
            dialogOpen: false,
        };
    }

    open(opt) {
        this.state.dialogOpen = true;
    }

    handleDialogClose() {
        this.state.dialogOpen = false;
    }
};

<lasso-page flags="ebayui-no-bg-icons"/>

<div>
    <div>
        <ebay-button class="open" priority="primary" on-button-click('open')>Open</ebay-button>
    </div>
    <ebay-dialog class='side-dialog' type="right" a11y-close-text="Close Dialog" aria-labelledby="dialog-title" open=state.dialogOpen on-dialog-close('handleDialogClose')>
        <p>A dialog</p>
    </ebay-dialog>
</div>
ianmcburnie commented 5 years ago

@ccinelli Does Dylan's answer resolve your issue? If so we can close this. If not, let's discuss.

ianmcburnie commented 5 years ago

Closing as this should no longer be relevant in CoreUI v3.0.0 (coming soon).