Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dialog): ability to specify dialog per open call #1978

Merged
merged 6 commits into from
May 19, 2024

Conversation

bigopon
Copy link
Member

@bigopon bigopon commented May 19, 2024

📖 Description

Addresses the need mentioned in #1671 and the dialog rfc at #1713 . This PR adds:

  1. the ability to specify a custom renderer, simply and inline in the dialogService.open call, like the following example:

    ...
    dialogService.open({
      template: '<div>hey there</div>',
      renderer: {
        render(host, settings) {
          const overlay = document.createElement('dialog-overlay');
          const contentHost = document.createElement('dialog-content-host');
          host.appendChild(overlay);
          host.appendChild(contentHost);
          return {
            overlay,
            contentHost,
            dispose() {
              host.removeChild(overlay);
              host.removeChild(contentHost);
            }
          }
        }
      }
    })

    Notice the object given to the renderer property, it should satisfy the interface of an IDialogDomRenderer:

    interface IDialogDomRenderer {
      render(dialogHost: Element, settings: IDialogLoadedSettings): IDialogDom;
    }
  2. an event manager extension point for custom event handling.

    With the ability to specify different renderers for different .open() call on the dialog service, it's becomes necessary to be able to have a flexible event handling strategy. So instead of leaving it hidden inside the dialog service & dialog controller, extract it into a separate block for extensibility. An example of byo dialog event manager is as follow:

    import { Registration } from 'aurelia';
    import { IDialogEventManager, IDialogController } from '@aurelia/dialog';
    
    export class MyDialogEventManager {
      static register(container) {
        Registration.singleton(IDialogEventManager, this).register(container);
      }
    
      add(controller: IDialogController): IDisposable {
        if (controller.settings.renderer === ...) {
          // manage this
        } else {
          // don't manage this
        }
    
        return {
          dispose: () => {
            // your disposal logic here
          }
        }
      }
    }

🎫 Issues

resolve #1671 - I'll be closing this, if you disagree with any points here and want changes, let's have a new PR
resolve #1713

@ekzobrain

  • with the addition of (2), your concern

keydown event is now listened on dialog's wrapper elements instead of Window (tabindex="-1" added to wrapper element to make it focusable and thus dispatch this event type). This allows not to warry about dialog hierarhy progmatically - the last open dialog (top) will always receive this event.

is addressed, as we can decide what to do with each controller in your own implementation, per application. This I think is better than assuming a tabindex on the overlay, as it requires the overlay to be focused in the first place, which won't always be the case.

The following 2 concerns

  • DialogService does not listen and react to dialog's wrapper keydown event, moved to DefaultDialogDomRenderer
  • DialogController does not listen and react to dialog's overlay click event, moved to DefaultDialogDomRenderer

are also addressed, as now you can always override the event handler with your own and add/remove event listeners the way you see fit. You can check how the default event listening is done in the default implementation.

feat(dialog): add keyboard manager for custom keyboard handling
Copy link

codecov bot commented May 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.56%. Comparing base (f1a73d6) to head (b4f7d7c).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1978      +/-   ##
==========================================
- Coverage   88.59%   88.56%   -0.03%     
==========================================
  Files         273      278       +5     
  Lines       22945    22892      -53     
  Branches     5305     5288      -17     
==========================================
- Hits        20328    20275      -53     
  Misses       2617     2617              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bigopon
Copy link
Member Author

bigopon commented May 19, 2024

Codecoverage is reporting uncovered lines on deleted code few commits ago, I'm not sure why.

@bigopon bigopon merged commit 7d44ed1 into master May 19, 2024
28 of 29 checks passed
@bigopon bigopon deleted the feat/dialog-open-renderer branch May 19, 2024 23:39
AureliaEffect pushed a commit that referenced this pull request May 23, 2024
2.0.0-beta.18 (2024-05-23)

**BREAKING CHANGES:**

* **dom-queue:** merge dom read and write queues (#1970) ([3a63cde](3a63cde))

**Features:**

* **dialog:** ability to specify dialog per open call (#1978) ([7d44ed1](7d44ed1))
* **dialog:** add event manager for custom event handling extension ([7d44ed1](7d44ed1))
* **kernel:** add last resolver (#1976) ([c47817c](c47817c))
* **router-lite:** current route (#1966) ([d966e15](d966e15))

**Bug Fixes:**

* **di:** use official metadata instead of weakmap (#1977) ([9aeeffa](9aeeffa))
* **router-lite:** current route subscription disposal (#1969) ([ace4c65](ace4c65))
* **convention:** typing: use array for bindables isntead of object (#1967) ([f1a73d6](f1a73d6))

**Refactorings:**

* **collection:** define map & set overrides on the instance instead of prototype (#1975) ([253e92a](253e92a))
* **runtime:** reoganise utils import ([253e92a](253e92a))
* **fetch-client:** extract error codes and cleanup (#1974) ([63ffdc9](63ffdc9))
* **i18n-validation:** replace errors with error codes (#1972) ([f91f31c](f91f31c))
* **dev:** turbo package input glob + ts dev script ([253e92a](253e92a))

**Docs:**

* **doc:** updated ISignaler documentation example (#1973) ([e0481d6](e0481d6))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rfc(dialog): ability to specify renderer per open call
1 participant