- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3
fix: collapsible section chevron bug, misc refactors [TDX-6738] #738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| Install the preview package from this PR@kong/spec-renderer@pr-738 | 
|  | ||
| <script lang="ts" setup> | ||
| import { ref, useAttrs, computed } from 'vue' | ||
| import { ref, useAttrs, computed, useTemplateRef } from 'vue' | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import { ref, useAttrs, computed, useTemplateRef } from 'vue' | |
| import { useAttrs, computed, useTemplateRef } from 'vue' | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const auth2ComponentRef = useTemplateRef<Array<InstanceType<typeof TryItAuth2>>>('auth2ComponentTemplate') | ||
| const auth2ClientCredentialsAuth = async (): Promise<Response | undefined> => { | ||
| if (!auth2ComponentRef.value?.[0].auth2ClientCredentialsAuth) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also remove generic type from here, as it causes type assertion. Let Vue pickup the type itself, as it also helped catch a bug here: we were expecting auth2ComponentRef.value?.[0] to always exist, and hence were directly accessing auth2ClientCredentialsAuth on it.
| const auth2ComponentRef = useTemplateRef<Array<InstanceType<typeof TryItAuth2>>>('auth2ComponentTemplate') | |
| const auth2ClientCredentialsAuth = async (): Promise<Response | undefined> => { | |
| if (!auth2ComponentRef.value?.[0].auth2ClientCredentialsAuth) { | |
| const auth2ComponentRef = useTemplateRef('auth2ComponentTemplate') | |
| const auth2ClientCredentialsAuth = async (): Promise<Response | undefined> => { | |
| if (!auth2ComponentRef.value?.[0]?.auth2ClientCredentialsAuth) { | 
| ref="model-property" | ||
| class="model-property" | ||
| ref="modelProperty" | ||
| class="modelProperty" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name shouldn't be changed.
| class="modelProperty" | |
| class="model-property" | 
| const expanded = ref<boolean>(true) | ||
| const toggleState = (e: Event) => { | ||
| if (!(e.target as HTMLElement).dataset.selectDropdownTrigger) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment here on why we need to do this
| useEventListener(popoverTrigger.value, 'mouseenter', showPopover) | ||
| useEventListener(popoverTrigger.value, 'focus', showPopover) | ||
| useEventListener(popoverTrigger.value, 'mouseleave', hidePopover) | ||
| useEventListener(popoverTrigger.value, 'blur', hidePopover) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can pass multiple events as an array to useEventListener, like this useEventListener(popoverTrigger.value, ['mouseenter', 'focus'], showPopover).
Can you try it out and verify?

Summary
Addresses: https://konghq.atlassian.net/browse/TDX-6738
Fix CollapsibleSection chevron icon bug when clicking on dropdown in CollapsibleSection title, it toggles the chevron icon (recordings).
Before
https://github.com/user-attachments/assets/f63c54ba-b4d7-4b87-bfad-6879f368007d
After
https://github.com/user-attachments/assets/3212c9a1-6b61-42a7-875f-0d99ccc5dc1d
Other misc refactors:
useTemplateRefin bunch of placesuseEventListenerinstead ofaddEventListenerwhere appropriate