[Issue-126] :: Implement Disclosure component #168
Conversation
|
Hi @NullVoxPopuli can you help me with the review for this PR |
| @@ -0,0 +1,14 @@ | |||
| <div | |||
There was a problem hiding this comment.
do we need the wrapping div?
|
|
||
|
|
||
| export default class Button extends Component { | ||
| guid = `${guidFor(this)}-tailwindui-disclosure-button`; |
There was a problem hiding this comment.
can we use the guid helper instead of having a backing class?
template-only components are more performant
| <Disclosure as |disclosure|> | ||
| <span class='rounded-md shadow-sm'> | ||
| <disclosure.Button | ||
| @index={{1}} |
There was a problem hiding this comment.
we do we have index here? A consumer should never need to manage the index, yeah?
| <button | ||
| type='button' | ||
| aria-haspopup={{true}} | ||
| aria-controls={{if @isOpen this.itemsGuid}} |
There was a problem hiding this comment.
can we generate a single GUID in the parent component, just to reduce the number of repeat guid creations?
passing a guid (and then concatting suffixes) allows us to then also get rid of data-test-selectors (we don't want to ship these to consumers either, but that's a current problem all over the addon that we need to fix -- out of scope for this PR)
| export default class Panel extends Component { | ||
| panelGuid = `${guidFor(this)}-tailwindui-disclosure-panel`; | ||
|
|
||
| @tracked isOpen = false; |
There was a problem hiding this comment.
why does this have its own isOpen state when the parent component also has isOpen?
In this PR, I have implemented the Disclosure component in ember similar to the react headless UI component
I have added the relevant tests for the component functionality as well
I have also added the component to the docs
Please find the screen shots of the implementation
