ember-inspector
ember-inspector copied to clipboard
add modifiers to render tree
Description
this adds modifiers and their html element to the render tree.
this also improves the UX for positional args, especially functions
this builds on #2549
Screenshots
this is ready. I tried to support all versions we currently check and also make it robust in case some things are not available. also, this is very close to the PR i have to glimmer-vm
@patricklx the code and tests look great here, thank you for taking this on! I was looking at the screenshot though, and shouldn't the modifier be inside the element? They seem to be outside of the closing >
.
It's by design, i didn't try to put it inside. Maybe it can be done, i can have a look tomorrow
I think it makes more sense to have it under the element for ux. Otherwise it can be too much on one line.
But i could add a >
after the last modifier of an element
@RobbieTheWagner i tried a few things, but was not able to do it in a good way. It could have multiple positional args and named args as well. maybe add some indicator that its inside an element? Or a smaller indent. Or larger indent?
I really think it should look the same as what users see in their code.
<li class="foo" {{did-insert this.myCoolFunction}}>
I do understand these could be rather long, but I think we should figure out a way to wrap things. Like essentially we want this markup to look how prettier would format it.
Wrapping is not good in this case, at least with CSS, since we are using vertical-collection which needs predictable/fixed row heights. I think . So it would need to be done manually. So everything would need to be in one line...
Also i often prefer to have it like this:
<li class="foo"
{{did-insert this.myCoolFunction}}
>
:)
how about this:
how about this:
This looks great! 👍
pushed the changes
@RobbieTheWagner what about this?
I found that this might need more work. The modifiers need to be removed from debug render tree when they get destroyed
@RobbieTheWagner ready
@patricklx looks like there are some conflicts. Would you mind resolving those please?
@RobbieTheWagner rebased