[DSS-225] Popover - add caret to popover #1637
Conversation
| &::after { | ||
| left: $-popover-tooltip-offset; | ||
| top: 100%; | ||
| transform: translate3d(-50%, 0, 0); | ||
| border-left: $-popover-tooltip-inactive; | ||
| border-right: $-popover-tooltip-inactive; | ||
| border-top: $-popover-tooltip-active; |
There was a problem hiding this comment.
Feels like most of this could move up to line 60 so we don't have to repeat it for each modifier, right?
There was a problem hiding this comment.
Much of this code isn't repeating as each modifier has different positional properties and 3 border sides updated. The only common thing I was able to pull out was
through 60. Is that what you're referring to?There was a problem hiding this comment.
Oh I see, is it possible to make the shape with the same three border declarations and then rotate it correctly or does that get funky with alignment?
There was a problem hiding this comment.
Oh I see, is it possible to make the shape with the same three border declarations and then rotate it correctly or does that get funky with alignment?
Love this (assuming the alignment/position doesn't get all out of whack with the transform).
There was a problem hiding this comment.
@goodwinchris I created this task for that work. The tooltip could also benefit from such change as well, so created a ticket for a shared function that both tooltip and popover would consume: https://kajabi.atlassian.net/browse/DSS-226
| content: ""; | ||
| position: absolute; | ||
| } | ||
|
|
||
| .sage-popover--is-expanded & { | ||
| visibility: visible; | ||
| } | ||
|
|
||
| .sage-popover--top & { |
There was a problem hiding this comment.
I believe this has been brought up before since it's an existing issue, but popovers on mobile have a high potential to break layouts. Though the caret addition helps with orientation, we should also consider standardizing display of all popovers on mobile, regardless of specified position.
Since we're only affecting the visibility on the popover, and its default position is right-aligned to its toggle/trigger, the popover's content tends to overflow the parent container on smaller screens. Even worse, left-positioned popovers can't be viewed/scrolled at all due to the overlay lock.
popover mobile overflow
popover truncated
Design will want to weigh in here, but treating popover content in the same way as a modal (centered on screen, fixed position) might be an option.
There was a problem hiding this comment.
@monkeypox8 created DSS-227 for tracking this work
| &::after { | ||
| left: $-popover-tooltip-offset; | ||
| top: 100%; | ||
| transform: translate3d(-50%, 0, 0); | ||
| border-left: $-popover-tooltip-inactive; | ||
| border-right: $-popover-tooltip-inactive; | ||
| border-top: $-popover-tooltip-active; |
There was a problem hiding this comment.
Oh I see, is it possible to make the shape with the same three border declarations and then rotate it correctly or does that get funky with alignment?
Love this (assuming the alignment/position doesn't get all out of whack with the transform).
- add caret to all popover instances - update the horizontal position based on presence of tooltip
QuintonJason
force-pushed
the
DSS-225_qj-add-caret-to-popovers
branch
from
8e10bac to
fef81f3
Compare
Description
create jira issue for border / rotation patternDSS-226Screenshots
Testing in
sage-libTesting in
kajabi-productsPopover. Minimal visual changeRelated
Closes DSS-225