fix: fade tray overlay backdrop on close#736
Conversation
🟡 Heimdall Review Status
🟡
|
| Code Owner | Status | Calculation | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| ui-systems-eng-team |
🟡
0/1
|
Denominator calculation
|
|
@hcopp would it be possible to get this merged? |
There was a problem hiding this comment.
Thanks for the ping @torkelrogstad, in the future you can reach out in #ask-cds to get a review faster 🙏🏻 .
You probably saw this already but we have this ready to merge we will need to bump changelogs, see https://github.com/coinbase/cds/blob/master/CONTRIBUTING.md#version-and-changelog for details.
| <motion.div {...motionProps} data-testid={`${props.testID}-motion`}> | ||
| {content} | ||
| <motion.div ref={forwardedRef} {...motionProps} data-testid={`${props.testID}-motion`}> | ||
| <VStack background="bgOverlay" onClick={onClick} pin="all" {...props} /> |
There was a problem hiding this comment.
I don't think we will be able to change the ref here @torkelrogstad.
While it would not have been recommended, someone could have been doing overlayRef.current?.classList.add('dimmed').
There was a problem hiding this comment.
@hcopp thanks for your review. I'm not a Coinbase employee, so I don't have access to what I assume is the private channel you're referring to. I've updated the implementation to take your feedback into consideration, and also updated the changelog.
7c59da5 to
0264c35
Compare
What changed? Why?
The tray overlay has a fade in effect, but prior to this commit did not have a fade out effect.
UI changes
Testing
How has it been tested?
Testing instructions
Change management
type=routine
risk=low
impact=sev5
automerge=false