Convert BsModal related classes to TS #2115
Conversation
| }} | ||
| {{else}} | ||
| {{#if (has-block)}} | ||
| {{yield}} | ||
| {{yield (hash title=Title close=Close)}} |
There was a problem hiding this comment.
I don't think we need to yield here. We are checking before of the block should receive parameters.
If Glimmer does not support that well, I think we should just remove the has-block-params code path. Simplifying the implementation may be good anyways. At least I don't see benefits in conditionally yuelding the component. I doubt that Embroider is able to detect it and tree-shake the child component if it's not needed.
| @@ -0,0 +1,5 @@ | |||
| export default function bsDefaultHelper<PRIMARY, SECONDARY>( | |||
There was a problem hiding this comment.
Please have a look at https://github.com/ember-bootstrap/ember-bootstrap/blob/2f0049c23d56a0b671f46981fb808ce7bcbe1797/ember-bootstrap/addon/helpers/bs-default.ts for a more advanced implementation, which does a null check as well.
| {{#let (ensure-safe-component (bs-default @buttonComponent (component "bs-button"))) as |Button|}} | ||
| <div class="modal-footer" ...attributes {{on "submit" (bs-default @onSubmit (bs-noop))}}> | ||
| {{#if (has-block)}} | ||
| {{yield}} | ||
| {{else}} | ||
| {{#if @submitTitle}} | ||
| <Button @onClick={{@onClose}}>{{bs-default @closeTitle "Ok"}}</Button> | ||
| <Button @type={{bs-default @submitButtonType "primary"}} onClick={{@onSubmit}} disabled={{bs-default @submitDisabled false}}>{{@submitTitle}}</Button> | ||
| <Button @type={{bs-default @submitButtonType "primary"}} @onClick={{@onSubmit}} disabled={{bs-default @submitDisabled false}}>{{@submitTitle}}</Button> |
There was a problem hiding this comment.
Good catch!
Is this causing a bug for consumers? If so, we may want to fix it in a separate PR and add missing test coverage.
There was a problem hiding this comment.
I've addressed this in #2123. The missing @ does not seem to be breaking anything, but it is nicer to have it there just to be sure.
SanderKnauff
force-pushed
the
convert-bs-modal-to-ts
branch
from
d63336a
to
b4287b7
Compare
SanderKnauff
changed the title
This commit includes: * BsModal::Header::Close * BsModal::Header::Title * BsModal::Body * BsModal::Footer * BsModal::Header
SanderKnauff
force-pushed
the
convert-bs-modal-to-ts
branch
from
b94b2ac
to
2f18c84
Compare
|
I've returned this PR to draft status as I found out I forgot some classes that need to be converted as well. |
WIP PR to track work of the conversion of BsModal and it's related components.
This also touches the
bs-defaultandbs-noophelpers.