feat: ion-datetime: supply modal role on default buttons

[eLyrin]

Prerequisites

• [X] I have read the Contributing Guidelines.
• [X] I agree to follow the Code of Conduct.
• [X] I have searched for existing issues that already include this feature request, without success.

Describe the Feature Request

The default buttons of ion-datetime close the nearest overlay without supplying a role (=undefined). When I await the onDidDismiss Promise I can't distinguish between submit and cancel. see here

As a workaround I would have to supply my own Buttons instead of using the default buttons.

Describe the Use Case

this.modalIonDateTime.present();
const dtResult = await this.modalIonDateTime.onDidDismiss();
// dtResult.role is undefined. Unable to proceed with follow-up logic

<ion-modal [keepContentsMounted]="true">
  <ng-template>
    <ion-datetime formControlName="ionDateTimeCtrl" [presentation]="type"
      [showDefaultButtons]="true" [locale]="(appLanguage.value | async)?.languageCode"
      [cancelText]="'GLOBAL_CANCEL' | translate" [doneText]="'GLOBAL_OK' | translate">
    </ion-datetime>
  </ng-template>
</ion-modal>

Describe Preferred Solution

No response

Describe Alternatives

No response

Related Code

No response

Additional Information

No response

[sean-perkins]

Hello @eLyrin thanks for this feature request!

When using the default buttons for cancel and confirm, you can use the ionCancel event for responding to the cancel operation and ionChange to listen for value changes as a result of clicking the confirm button.

Simplified example: https://stackblitz.com/edit/n5zmwm?file=index.html

In your use case, you would not subscribe to the dismiss state of the overlay. You would bind the respective listeners to the ion-datetime to perform whatever operations you expect when it is a confirmed value vs. the user canceling date selection. This connects your logic closer to the area responsible.

Can you share if these events do not satisfy your use case and if not, why?

Thanks!

[eLyrin]

Hey @sean-perkins and thanks for your reply.

I noticed that I need custom buttons no matter what (because I need a "Today"-Button).

your suggestion would also work. But it's more cumbersome than simply awaiting the onDidDismiss Promise and it's also more error prone imo:

const role = await firstValueFrom(merge(
  this.ionDateTime.ionCancel.pipe(map(() => "cancel" as const)),
  this.ionDateTime.ionChange.pipe(map(() => "submit" as const)),
  // Error prone: easy to forget the "backdrop" case
  // Also when Ionic would one day call dismiss before the other events, this code would lead to bugs
  this.modalIonDateTime.didDismiss.pipe(map(() => "backdrop" as const))
));

For me it would be better DX if the role of onDidDismiss would be set - just as you already do for role: "backdrop"

Another thing I noticed: when you select a date, hit cancel, and then open the calendar again..., the previously canceled date is highlighted instead of the actual FormControl value (Angular).

[liamdebeasi]

Hey there,

Apologies for the delay in response. We think this would be a good functionality to have in Ionic. Any interest in contributing a fix to implement this? We will need to do the following to implement the behavior:

1. Update closeParentOverlay to accept a role: string parameter. This should be passed to the modal/popover as popoverOrModal.dismiss(undefined, role) (the first parameter is optional data which we do not have here).
2. Update Datetime's confirm method to pass 'datetime-confirm' to this.closeParentOverlay.
3. Update Datetime's cancel method to pass 'datetime-cancel' to this.closeParentOverlay.

Note that the datetime-confirm or datetime-cancel naming may change during review. I'd recommend assigning these values to consts at the bottom of datetime.tsx:

const CANCEL_ROLE = 'datetime-cancel';
const CONFIRM_ROLE = 'datetime-confirm';

---

As far as tests go, we'll want to test that both the confirm and cancel roles are being passed correctly. I recommend making a new directory in components/datetime/test for this. The following tests should be sufficient:

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ config, title }) => {
  test.describe(title('datetime: overlay roles'), () => {
    test.beforeEach(async ({ page }) => {
      await page.setContent(`
        <ion-modal>
          <ion-datetime></ion-datetime>
        </ion-modal>  
      `, config);
    });
    test('should pass role to overlay when calling confirm method', async ({ page }) => {
      const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
      const modal = page.locator('ion-modal');
      const datetime = page.locator('ion-datetime');
      
      await modal.evaluate((el: HTMLIonModalElement) => el.present());

      await datetime.evaluate((el: HTMLIonDatetimeElement) => el.confirm(true));

      await ionModalDidDismiss.next();
      expect(ionModalDidDismiss).toHaveReceivedEventDetail({ data: undefined, role: 'datetime-confirm' });
    });
    test('should pass role to overlay when calling cancel method', async ({ page }) => {
      const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss');
      const modal = page.locator('ion-modal');
      const datetime = page.locator('ion-datetime');
      
      await modal.evaluate((el: HTMLIonModalElement) => el.present());
      
      await datetime.evaluate((el: HTMLIonDatetimeElement) => el.cancel(true));
      
      await ionModalDidDismiss.next();
      expect(ionModalDidDismiss).toHaveReceivedEventDetail({ data: undefined, role: 'datetime-cancel' });
    });
  });
});

---

Once a PR is up for this I can assist with any documentation.

[ionitron-bot[bot]]

This issue has been labeled as help wanted. This label is added to issues that we believe would be good for contributors.

If you'd like to work on this issue, please comment here letting us know that you would like to submit a pull request for it. This helps us to keep track of the pull request and make sure there isn't duplicated effort.

For a guide on how to create a pull request and test this project locally to see your changes, see our contributing documentation.

Thank you!

[Rahul-28]

Hi, I would like to work on this, can you assign this issue to me

[liamdebeasi]

@Rahul-28 Thanks for volunteering to work on this! I've assigned the task to you. Instructions on what we need to change (including tests to write) are documented in https://github.com/ionic-team/ionic-framework/issues/28298#issuecomment-1821792099. Please let me know if there's any way I can be of assistance!

[Rahul-28]

Hey @liamdebeasi the task is done, but few tests are failing even though I didn't work on them. would it be safe to raise a PR? if not please suggest a way to fix them

[liamdebeasi]

Yeah, you can open the PR. We can discuss the failing tests on the PR thread.

[Rahul-28]

hey @liamdebeasi , I have opened a PR #29221 , please check it out

[liamdebeasi]

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/29221, and the feature will be available in an upcoming release of Ionic Framework.

[ionitron-bot[bot]]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.