-
Notifications
You must be signed in to change notification settings - Fork 128
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
Refactor SidePanel #2903
base: next
Are you sure you want to change the base?
Refactor SidePanel #2903
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Size Change: +3.25 kB (+0.48%) Total Size: 676 kB
ℹ️ View Unchanged
|
bd6ce6a
to
762ceb9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2903 +/- ##
==========================================
+ Coverage 87.98% 88.08% +0.09%
==========================================
Files 227 221 -6
Lines 13095 12890 -205
Branches 1794 1788 -6
==========================================
- Hits 11522 11354 -168
+ Misses 1519 1483 -36
+ Partials 54 53 -1
|
762ceb9
to
2843340
Compare
2843340
to
714716c
Compare
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 the first half of my review that focuses on the changes in the Dialog component (and issues I had missed in previous reviews 🙈).
I'll review the SidePanel related changes next.
packages/circuit-ui/components/SidePanel/components/Header/Header.module.css
Show resolved
Hide resolved
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.
I noticed you renamed a number of props, but I assume only the internal API has changed and the public API is unchanged, correct?
I spotted a bug when resizing the viewport from mobile to desktop while the side panel is open. The dialog remains positioned in the center of the viewport.
packages/circuit-ui/components/SidePanel/SidePanelContext.spec.tsx
Outdated
Show resolved
Hide resolved
8b37d02
to
6b3c420
Compare
4d0c3ee
to
7fe7e3a
Compare
c1419a9
to
7e17a25
Compare
Addresses DSYS-878
Purpose
Refactor the SidePanel component to replace react-modal with the Dialog component. Remove the react-modal dependency.
Approach and changes
backButtonLabel
react-modal
packagepreventClose
prop into :preventOutsideClickClose
,preventEscapeKeyClose
andhideCloseButton
.Definition of done