pgbross / vue-material-adapter

Vue wrapper arround Material Components for the Web
ISC License
105 stars 20 forks source link

The default dialog button is not detected correctly #185

Closed axxie closed 1 year ago

axxie commented 3 years ago

Description

The isDefault prop on the mcw-dialog-button doesn’t really work.

Steps to reproduce

  1. Create the dialog with the text field, and make sure the isDefault prop is set on the second dialog button, such as below:

    <mcw-dialog v-model="isOpen">
    <mcw-dialog-title>Demo dialog</mcw-dialog-title>
    <mcw-dialog-content>
      <mcw-textfield v-model="text" label="Text"/>
    </mcw-dialog-content>
    
    <mcw-dialog-footer>
      <mcw-dialog-button action="dismiss">Cancel</mcw-dialog-button>
      <mcw-dialog-button action="ok" isDefault @click="accept">Ok</mcw-dialog-button>
    </mcw-dialog-footer>
    </mcw-dialog>
  2. Add some log to the accept function

  3. Open the dialog and focus the text field

  4. Press the Enter key

Expected result

  1. The accept function is called (there is a log)
  2. The dialog is closed

Actual result

  1. The accept function is NOT called (there is no log)
  2. The dialog is closed

Analysis

It looks like the issue is caused by the implementation of getting the default button element:

https://github.com/pgbross/vue-material-adapter/blob/8058d8b06b805de3e66f81b6eeb78646d599ec7c/src/dialog/dialog.js#L153-L155

Due to how vue attribute binding works, all dialog buttons have the data-mdc-dialog-button-default attribute. The default button has it set to true, and other buttons have it set to false. However, the querySelector functions ignores the value of the attribute and returns the first element that have it, effectively always returning the first button.

Workaround

Make the default button the first in the dialog.

pgbross commented 1 year ago

Fixed in PR#193