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

Add the ModalManager utility #625

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sukima
Copy link
Contributor

@sukima sukima commented Sep 19, 2022

A utility to manage modal dialog boxes. It provides an isOpen tracked property that can be used in templates to show/hide a modal dialog.

This utility helps the code logic by offering a promise that will result in the conclusion of the modal dialog. It also allows you to provide the dialog component a simple manager interface that it can use to confirm, cancel, and even reject based on user input. That result can then be used for control flow by that initiator or even passed around as a promise as needed.

import Component from '@glimmer/component';
import { ModalManager } from 'ember-resources/utils/modal-manager';

class Demo extends Component {
  manager = new ModalManager();
  openAndManageModal = async () => {
    let { reason, value } = await this.manager.open();
    if (reason !== 'confirmed') return;
    doSomethingWith(value);
  }
}
<button type="button" {{on "click" this.openAndManageModal}}></button>
{{#if this.manager.isOpen}}
  <MyModal @manager={{this.manager}} />
{{/if}}

@sukima
Copy link
Contributor Author

sukima commented Sep 19, 2022

I think the tests are missing the generic type like they should but my editor didn't catch it because it could not import anything from ember-resource because I could not get the project to build anything. This PR needs work. But the bulk of the idea/logic is there and I'm using the API provided in other projects.

Copy link
Owner

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the submission! It's exciting to see a new util added!

A couple todos tho

  • we need an exports entry, like here
    this is because we put the built files in dist, so we need to tell node/npm/etc where those files are located

  • we need a typesVersions entry, like here
    this is mostly for supporting older typescript versions -- as soon as we drop compatibility with TS 4.5 and 4.6, we have fun way to use exports to specify the type declaration file

  • The utility itself isn't a resource -- it's good to have this class, and probably what a resource utility will use, but we'll need to create that wrapper glue.
    Here is an example of wrapper glue RemoteData -- this wrapper glue is entirely to provide alternate APIs / function overloads, and could likely be optional here -- but the non-optional most important bit, is reactivity. The function surrounding resource is allows exploration of different APIs, like for what you'd use in the template. The reactivity part is the most important though -- for example, if we're wanting to manage the "open" status externally, we can define what we want to do when that external open value changes to false, if it started as true.

  • The tests will need to be updated to use the wrapper glue -- ideally, we shouldn't see new ModalManager() in the tests, because it'll become private api -- we want to reduce the number of sharp edges folks could have when interacting with APIs from this library (as there are already a good number, as resources are a new concept for a good few folks!)

Again, thanks for the submission! I think this is really close!

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.

2 participants