Update table of contents component to glimmer #809
Conversation
Let's avoid passing the 'projectId' argument to the TableOfContents component as it does not do anything with it.
Let's update the TableOfContents component to extend glimmer/component instead of ember/component. Glimmer components offer a bunch of various advantages over classic components and are the direction that ember is heading in general.
Right now, we mix both single quotes and double quotes throughout the template file. Let's prefer single quotes instead.
Let's prefer angle bracket syntax to curly syntax when invoking the TableOfContents component in our integration tests. This will fix deprecation warnings about using the latter, which make the test results a bit harder to decipher.
| @@ -0,0 +1,44 @@ | |||
| <ol class='toc-level-0'> | |||
There was a problem hiding this comment.
One note: this is not going to have the exact same DOM as we had before, in two ways:
- It won't accept
...attributes, which@ember/componentimplicitly did. - It won't have a wrapping
div.
In this case, that should be fine: the rendering doesn't depend on the wrapping element (I checked!), but I wanted to flag it up for anyone who reads this PR later.
| isShowingNamespaces=true | ||
| }} | ||
| <TableOfContents | ||
| @showPrivateClasses={{true}} |
There was a problem hiding this comment.
Note for a follow-on: this currently uses two-way binding between an Input in the implementation and a property on a service very far away. As a follow-up, we'll want to update that to actually just use normal event handling.
| {{/if}} | ||
|
|
||
| <li class='toc-level-0'> | ||
| <a {{action 'toggle' 'classes'}} href='#' data-test-toc-title='classes'>Classes</a> |
There was a problem hiding this comment.
It is good that these didn't change in this case, but we'll definitely want to switch to {{on}} in a follow-up.
| </li> | ||
| </ol> | ||
| <label class='toc-private-toggle'> | ||
| <Input @type='checkbox' @checked={{@showPrivateClasses}} class='private-deprecated-toggle' /> |
There was a problem hiding this comment.
And this is the two-way binding we'll want to replace.
|
This specific CI failure is happening on |
Let's update the TableOfContents component to extend glimmer/component instead of ember/component. Glimmer components offer a bunch of various advantages over classic components and are the direction that ember is
heading in general.
While we're here, let's prefer single quotes to double quotes throughout our template file and cleanup any unused arguments into this component.
Finally, let's make sure that we're using angle bracket syntax rather than curly brace syntax to invoke the component in our tests as this cleans up some deprecations logged when they are run.
This should resolve the
app/components/table-of-contents.jssection of #805.