-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat(OnyxTooltip): auto align tooltip #1821
base: main
Are you sure you want to change the base?
Conversation
ChristianBusshoff
commented
Sep 2, 2024
- add auto alingment to the tooltip component
- add screenshot-tests
- add a demo on the homeView
|
please adjust the PR title to the convention :) the pattern is
|
packages/sit-onyx/src/components/OnyxTooltip/OnyxTooltip.stories.ts
Outdated
Show resolved
Hide resolved
This is an auto-generated pull request. All Playwright screenshots have been updated. Co-authored-by: ChristianBusshoff <[email protected]>
This is an auto-generated pull request. All Playwright screenshots have been updated. Co-authored-by: ChristianBusshoff <[email protected]>
@@ -31,7 +30,7 @@ export const useOpenDirection = (element: MaybeRef<Element | undefined>) => { | |||
if (!element) return undefined; | |||
|
|||
const style = getComputedStyle(element); | |||
if (style.overflow === "hidden") { | |||
if (style.overflow === "hidden" || style.overflow === "hidden auto") { |
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 (style.overflow === "hidden" || style.overflow === "hidden auto") { | |
if (style.overflow.includes(hidden)) { |
/** | ||
* How to align the tooltip relative to the parent element. | ||
*/ | ||
align?: TooltipAlignment; |
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.
align?: TooltipAlignment; | |
alignment?: TooltipAlignment; |
In the OnyxSelect we currently call it alignment
, would be great if we can keep it identical
onMounted(() => { | ||
const updateOnResize = () => { | ||
updateOpenDirection(); | ||
updateWedgePosition(); | ||
}; | ||
|
||
window.addEventListener("resize", updateOnResize); | ||
|
||
// initial update | ||
updateOpenDirection(); | ||
updateWedgePosition(); | ||
}); | ||
|
||
onBeforeUnmount(() => { | ||
window.removeEventListener("resize", updateOpenDirection); | ||
window.removeEventListener("resize", updateWedgePosition); | ||
}); |
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.
onMounted(() => { | |
const updateOnResize = () => { | |
updateOpenDirection(); | |
updateWedgePosition(); | |
}; | |
window.addEventListener("resize", updateOnResize); | |
// initial update | |
updateOpenDirection(); | |
updateWedgePosition(); | |
}); | |
onBeforeUnmount(() => { | |
window.removeEventListener("resize", updateOpenDirection); | |
window.removeEventListener("resize", updateWedgePosition); | |
}); | |
const updateDirections = () => { | |
updateOpenDirection(); | |
updateWedgePosition(); | |
}; | |
useGlobalEventListener({ | |
type: "resize", | |
listener: () => updateDirections(), | |
}); | |
// initial update | |
onMounted(() => updateDirections()); |
We have a utility from other @sit-onyx/headless
package for this which already covers the unMounted / unmount part :)
[`onyx-tooltip--align--${wedgePosition.value}`]: props.align === "auto", | ||
[`onyx-tooltip--align--${props.align}`]: props.align !== "auto", |
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.
[`onyx-tooltip--align--${wedgePosition.value}`]: props.align === "auto", | |
[`onyx-tooltip--align--${props.align}`]: props.align !== "auto", | |
[`onyx-tooltip--${wedgePosition.value}`]: props.align === "auto", | |
[`onyx-tooltip--${props.align}`]: props.align !== "auto", |
in BEM there is only 1 --
allowed, so we need to rename it e.g. like this
/** | ||
* Recursively finds the first parent element with hidden overflow. | ||
*/ | ||
const findParentWithHiddenOverflow = (element?: Element): Element | undefined => { |
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.
This is copied from the useOpenDirection
composable, right? So we could export it there to not duplicate code