Closed crookse-cl closed 10 months ago
Comments on topics raised:
TODO on line 95 in Modal.tsx: Yes, we should add that as an issue to fix because it happens in the package already
Reduced Motion: Nice feat to add. Would love to see some OSS contrib there 😊
Mobile viewport orientation: We can talk about choices. Also an issue that could be added to the repo.
Summary
I noticed the
Modal
component is blurry on some screens while working with theConnectButton
component. Looked into it and noticed it uses transform CSS properties. Although using these properties in conjunction with Framer Motion are probably more performant than CPU-rendered methods, I'm proposing we move theModal
component away from transform CSS properties and use a combination of width, flexbox, and position CSS properties to create the same effect, but with a higher level of accessibility. Thoughts?Changes made
BackgroundLayer
andWrapper
into a singleModalScreen
componentWrapper
toModalAnimator
discussed in the next bullet belowModalAnimator
to only provide animations for the modalModalContents
component which provides the styles that theWrapper
component provided (background color, border radius, etc.)modal-screen
) to identify them faster in dev tools. Wondering if these names should be more unique?Cons to the above changes
Modal
changes from desktop to mobile version on window resize events, the animation isn't as smooth as it is in the original implementation. However, my argument for this being an ok thing is users aren't really going to be resizing their windows and seeing this jumpiness from desktop to mobile version all the time.How to test
ConnectButton
component from the built libmain
branchQuestions
Modal.tsx
file. I see the opacity stops at 0.4, but it's causing the opacity animation to pause. Wondering if the 0.4 value is intentional as a workaround for something, so I added a comment instead of changing the value.ConnectButton
component and immediately click theBackgroundLayer
component when it appears.hidden
variant is applied at the same time theshown
variant is running and it causes theModal
to stay on the screen with a 0.4 opacity.Other topics for discussion
Modal
component's mobile version is triggered on window width. This means mobile devices oriented in landscape mode might not trigger the mobile version which could cause theModal
component to go outside the bounds of the viewport. Maybe we look at:Modal
component so that it only works in portrait mode;Modal
to be the width and height of the entire viewport at specific heights/widths; orModal
component more cross-compatible and/or feeling more native to the device it's being shown on