Create block interactive template (discarded)#52549
Conversation
juanmaguitar
requested review from
ajitbohra,
fabiankaegy,
gziolo,
ndiego and
ryanwelcher
as code owners
ryanwelcher
left a comment
ryanwelcher
left a comment
There was a problem hiding this comment.
The template code looks great and I love the addition of viewScript. Before we merge, not sure if we need another review in regards to this adding a new package.
|
cc @gziolo |
|
@juanmaguitar it looks like we need to update the Create Block test to account for the new |
luisherranz
left a comment
luisherranz
left a comment
There was a problem hiding this comment.
Hey, Juanma. The template code looks good to me but I don't understand why this PR is built on top of a branch that contains part of the variant PR instead of trunk. It's not clear to me what's going to be merged and why it will end up in a new branch. From the variant PR, we only need the viewScript property propagation.
Would it be worth switching the base of this PR to trunk so we can clearly see what's included? Thanks!!
| @@ -0,0 +1,48 @@ | |||
| {{#isBasicVariant}} | |||
There was a problem hiding this comment.
I see double-line breaks in this file. Is it on purpose?
|
|
||
| toggle: ( { context } ) => { | ||
|
|
||
| context[ '{{namespace}}' ].isOpen = !context[ '{{namespace}}' ].isOpen; |
There was a problem hiding this comment.
| context[ '{{namespace}}' ].isOpen = !context[ '{{namespace}}' ].isOpen; | |
| context[ '{{namespace}}' ].isOpen = ! context[ '{{namespace}}' ].isOpen; |
There's a missing space after the ! (my fault).
|
I'm closing this PR in favor of this other PR #52612 |
@luisherranz You're right. That PR was an attempt of using the same branch and the same PR fo these changes. |
|
Thank you, Juanma. I've reviewed the other PR 🙂 |
juanmaguitar
changed the title
3 participants
What?
Creation of the "create-block-interactive-template" and addition of "view.js" to the default "create-block"
Why?
For the reasons explained here