-
Notifications
You must be signed in to change notification settings - Fork 21
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
refactor: Migrate preselected filters component #1422
refactor: Migrate preselected filters component #1422
Conversation
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
…ted-filters.vue Co-authored-by: Guillermo Cacheda <[email protected]>
…ted-filters.vue Co-authored-by: Guillermo Cacheda <[email protected]>
…ted-filters.vue Co-authored-by: Guillermo Cacheda <[email protected]>
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
const eventMetadata = { | ||
moduleName: null, | ||
location: 'none', | ||
replaceable: true | ||
}; | ||
|
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.
If you don't care what the metadata is in the tests, you can use expect.any(Object)
.
Remove this and use that instead.
packages/x-components/src/x-modules/facets/components/__tests__/preselected-filters.spec.ts
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/facets/components/preselected-filters.vue
Outdated
Show resolved
Hide resolved
return () => useNoElementRender(slots); | ||
} |
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.
Use the render
function instead, it's more declarative.
return () => useNoElementRender(slots); | |
} | |
}, | |
render() { | |
return useNoElementRender(this.$slots) | |
} |
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.
Hi, if I'm not wrong render is only available if we use the Options API, for the Composition API we should use h
and return it directly, which I think is almost the same we are doing now.
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 defineComponent
function receives an options object that is more or less the same type of object you create in the options API (you are using it already, passing the props and setting up a name). render
is one of the options that you can send to defineComponent
and will work with setup
when setup
doesn't use return
…ted-filters.vue Co-authored-by: Guillermo Cacheda <[email protected]>
…_/preselected-filters.spec.ts Co-authored-by: Guillermo Cacheda <[email protected]>
…ted-filters.vue Co-authored-by: Guillermo Cacheda <[email protected]>
…ted-filters.vue Co-authored-by: Guillermo Cacheda <[email protected]>
return () => useNoElementRender(slots); | ||
} |
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 defineComponent
function receives an options object that is more or less the same type of object you create in the options API (you are using it already, passing the props and setting up a name). render
is one of the options that you can send to defineComponent
and will work with setup
when setup
doesn't use return
Pull request template
Migrate the
PreselectedFilters
component to Vue 2.7Motivation and context
Type of change
What is the destination branch of this PR?
Main
How has this been tested?
Tests performed according to testing guidelines:
Checklist:
TEST
Check the component is working as expected: