Implement SVG-based modal mode #248
Conversation
BrianSipple
force-pushed
the
4.0-modal-refactor--svg-solution
branch
from
49d6fcb
to
ff2a1f4
Compare
BrianSipple
requested review from
RobbieTheWagner and
chuckcarpenter
and removed request for
RobbieTheWagner
|
It looks like the sizing of the opening is slightly off on some elements. I'm not sure if this is something we can control or not. Also, is there any way we could make masks of the text in the element to avoid the box entirely? This is definitely an elegant solution for working with nested elements, but I prefer the aesthetics of just having the text highlighted, as it was before. |
| if (targetElement) { | ||
| positionModalOpening(targetElement, modalOverlayOpening); | ||
|
|
||
| this._onScreenChange = () => { |
There was a problem hiding this comment.
I don't understand why a private function starts empty and gets filled in here?
There was a problem hiding this comment.
My goal here is to update this._onScreenChange before calling addEventListener with it. I'm open to ideas on making this a bit cleaner.
|
Would be great to have docs on the functions as well. Make doc automation later easier. |
|
@rwwagner90 It looks like the selection is occurring that way because we're currently targeting the container of the heading in the dummy app:
Here's what we get when we target the
|
BrianSipple
force-pushed
the
4.0-modal-refactor--svg-solution
branch
from
ff2a1f4
to
5136a3c
Compare
Closes #197