Create custom dialog for mdc-dialog component

[ggalli]

I've been working with ember-cli-mdc-dialog and showed up the necessity to use this component with custom action buttons. I created a new component "mdc-custom-dialog" where it is possible to pass whatever we want.

[hilljh82]

@ggalli Thanks for taking time to submit this PR.

I do have several question(s), and some feedback on the implementation. The first and most important question is what value does this provide over the current MdcDialog component? For example, what is missing that warrants creation of a completely new component? Unfortunately, it's not clear from the PR.

[ggalli]

@hilljh82 Thanks for reply.

The main reason is to have more control over the buttons, for example, styles, events, loading state, etc. I've seen in React Material Web Components lib that they follow this approach of having two components. I can improve the implementation if you have feedbacks and suggestions.

[hilljh82]

@ggalli

I've seen in React Material Web Components lib that they follow this approach of having two components.

Ok. FWIW, with this library, we are not really concerned about React Material Web Components approach because we take a completely different approach to our design.

The main reason is to have more control over the buttons, for example, styles, events, loading state, etc.

The styles should be handled via css. I would recommend looking at ember-cli-styled to help modularize your styles. This add-on helps us manage styling dialogs at the application level, page/route level, and component level.

I, however, can see the advantage of this component for improving "action" support on buttons, and the various states of the dialog. The approach, however, needs to be changed/modified. Right now, there is little code reuse in the design. Also, we need additional components to support this design approach. Lastly, I don't see the advantage named blocks in this example since we can achieve the same functionality without named blocks.

Here is my recommendation (at a high-level).

1. The current MdcDialog can be renamed to MdcSimpleDialog.
2. The MdcSimpleDialog contains the MdcDialog class. This means the MdcSimpleDialog is an aggregate class. So, we just need to map the attributes of MdcSimpleDialog to MdcDialog within the handlebars code.
3. Your approach for setting the classes on the buttons is not correct. You are mixing non-EmberJS approaches with EmberJS approaches.
4. Introduce <MdcDialogButtons> and <MdcDialogButton> components. This components should subclass from the appropriate buttons already in the framework. Also, the developer should not have to know what "data-*" properties to set. This is considered a low-level detail. These are Wrapper Facade classes. Their intent is to shield the developer from the underlying details of the subsystem. Also, this will fix the issues in how you are defining the buttons.

Here is an example of using the newly proposed MdcDialog and MdcSimpleDialog:

<MdcDialog ...>
  <MdcDialogContent>
    This is the content of the dialogs. I prefer this design approach over the named blocks.
  </MdcDialogContent>

  <MdcDialogActions>
    <MdcDialogAction @label="Ok" @action="ok" {{on "click" this.accept}} />
    <MdcDialogAction @label="Cancel" @action="cancel" {{on "click" this.cancel}} />
  </MdcDialogActions>
<MdcDialog>

<MdcSimpleDialog>
  This is the old MdcDialog. We need this so people can just to a search 
  and replace to ensure the dialogs do not break.
</MdcSimpleDialog>

[ggalli]

@hilljh82 I totally agree with you. I will make this changes an submit a new PR. Thanks to understand and feedbacks, it's my first PR in an open source project haha.

[hilljh82]

@hilljh82 I totally agree with you. I will make this changes an submit a new PR. Thanks to understand and feedbacks, it's my first PR in an open source project haha.

Well, congrats on taking time to make your first PR in an open-source project. It's a first major step for you on a hopefully a long and rewarding career in Software Engineering. Now, let's see if we can get this (re)designed correctly! :)

[ggalli]

Hi @hilljh82

I pushed new changes in this branch refactoring the implementation with your feedbacks. Can you check if it`s ok?

[hilljh82]

@ggalli Thank for taking the time to make the requested changes. I am going to hold off on merging this into master/main. I plan to include it in ember-cli-mdc@3.0.0 because the new

component will be a breaking change, which I'm ok with.