fix(dialog): prevent close when click released in scrim

[vdegenne]

This is an attempt to avoid the dialog to close when the mouse click was initiated inside the content but released from the scrim.

[asyncLiz]

Can we swap this to pointerdown?

[vdegenne]

And preserving the existing click event you mean?

[vdegenne]

I swapped to pointerdown and renamed the flag so it reads better.

[VandeurenGlenn]

Doesn't click return the target where mouse up happens?

[asyncLiz]

Update on this, there's some screenshot test failures that need more investigating, so apologies for the delay

[vdegenne]

Update on this, there's some screenshot test failures that need more investigating, so apologies for the delay

Can you give me more details? the tests run fine on my end.

[asyncLiz]

Update on this, there's some screenshot test failures that need more investigating, so apologies for the delay

Can you give me more details? the tests run fine on my end.

It's internal tests, so they don't run on github 😔

[AndrewJakubowicz]

I haven't had a chance to dig into it, but the failing test looks something like this: https://lit.dev/playground/#gist=62443e4343a872dc51982907ea6dfc9d

  <md-dialog open>
       <div slot="headline">Dialog</div>
        <div slot="content" style="height: 500px;">
          <md-filled-select menu-fixed>
            <md-select-option headline="Apple" value="apple"></md-select-option>
            <md-select-option headline="Apricot" value="apricot"></md-select-option>
          </md-filled-select>
        </div>
        <div slot="actions">
          <md-filled-button>Cancel</md-filled-button>
        </div>
  </md-dialog>

The failing test is that clicking the md-filled-select after this change doesn't open the select, when it expects the md-filled-select to open.

[vdegenne]

I noticed quite a few quirks on the select element (not just in a dialog), maybe it's a good opportunity to polish this element instead of trying to fix one problem related to this specific case.

This is the first one I spotted: #4915

[e111077]

the failing screenshot test seems to be weird. I can't nail down the exact issue but looks flaky when attempting to reproduce the issue with a debugger in a live environment in the test. Otherwise, normal interaction looks good. Gonna have to spend a bit of time to figure out what's going on here

[austinw-fineart]

I've got a failing case that's easy to reproduce:

1. Put a checkbox in the dialog
2. Toggle the checkbox with the Spacebar
3. Watch it close the dialog

Not every click event has a preceding pointerdown event so the next click will fail the check. You should probably just handle both events.

[vdegenne]

@austinw-fineart This looks like a different issue to me, plus I couldn't reproduce it.
If you have a problem could you open another issue and join a playground to it?
You can use this template: https://lit.dev/playground/#gist=db372518cfa9cd6097b7842e6da5f740
Thanks

[austinw-fineart]

@vdegenne I'm afraid I do not know how to import this into playground. I ran the test case on this pull request.