-
Notifications
You must be signed in to change notification settings - Fork 16
feat(drawer): mouse move and css units management #1249
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
base: main
Are you sure you want to change the base?
Conversation
a4491e0 to
a3c3922
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1249 +/- ##
==========================================
- Coverage 90.61% 85.82% -4.80%
==========================================
Files 117 50 -67
Lines 3399 2349 -1050
Branches 545 420 -125
==========================================
- Hits 3080 2016 -1064
- Misses 205 227 +22
+ Partials 114 106 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2e4dcf3 to
e1d38d2
Compare
95764c9 to
083426b
Compare
0eca38d to
5abf3ac
Compare
| encapsulation: ViewEncapsulation.None, | ||
| }) | ||
| export default class SizesDrawerComponent { | ||
| width = '20rem'; |
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.
not needed anymore
| --#{$prefix}drawer-size: max-content; | ||
| --#{$prefix}drawer-min-size: 0; | ||
| --#{$prefix}drawer-max-size: none; | ||
| --#{$prefix}drawer-splitter-size: 8px; |
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.
it seems to be replaced by $splitter-size
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.
True. Finally, I changed it for the css version, as this is something that we may want to customize.
core/src/components/drawer/drawer.ts
Outdated
| }, | ||
| })); | ||
| onMoveEnd() { | ||
| const newSize = drawerElement$()![isVertical$() ? 'offsetHeight' : 'offsetWidth']; |
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.
maybe we can already add a bit of logic when the currentSize gets to the minimum width (or some specified value) - we close the drawer? Because currently we can drag it all the way to the left and it leaves only a tiny bit of splitter out
| maxSize = element.getBoundingClientRect()[dimension] + 'px'; | ||
| context.maxSize = maxSize; | ||
| } | ||
| // if (!maxSize) { |
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.
still need the commented code?
core/src/utils/writables.ts
Outdated
| }; | ||
|
|
||
| /** | ||
| * A writable with default options that normalizes its value to a boolean or null. |
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.
typo in the comments:, has to be: number or null
4ca25da to
28ac809
Compare
|
|
||
| @Component({ | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| imports: [SlotDirective, DrawerStructureDirective, UseDirective], |
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.
UseDirective is not needed anymore here
45729e3 to
d6efee5
Compare
core/src/components/drawer/drawer.ts
Outdated
| onMoveStart() { | ||
| const isVertical = isVertical$(); | ||
| viewportSize = window[isVertical ? 'innerHeight' : 'innerWidth']; | ||
| const drawerElement = drawerElement$(); |
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.
probably we can avoid having the if and directly go with drawerElement$()! as in moveEnd as it is always there (+saves coverage :D)
d6efee5 to
0a48438
Compare
No description provided.