-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(column-resize): Resize table as well as columns, improve handing … #19264
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
Conversation
…of min/max width, and simplify overlay positioning.
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.
Also general function JsDocs
@andrewseguin PTAL
@@ -61,6 +63,10 @@ export abstract class ColumnResize implements AfterViewInit, OnDestroy { | |||
return `cdk-column-resize-${this.selectorId}`; | |||
} | |||
|
|||
setResized() { | |||
this.elementRef.nativeElement!.classList.add(WITH_RESIZED_COLUMN_CLASS); |
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.
Why does this need to be called from other classes instead of only responding to the resize stream?
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 went back and forth on this - I was not sure that applying a min/max size should trigger the notifier, which will eventually have side effects like persisting the size setting.
src/components-examples/material-experimental/column-resize/opt-in/opt-in-column-resize-demo.ts
Outdated
Show resolved
Hide resolved
...es/material-experimental/column-resize/default-enabled/default-enabled-column-resize-demo.ts
Outdated
Show resolved
Hide resolved
src/cdk-experimental/column-resize/column-resize-directives/column-resize-flex.ts
Outdated
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.
LGTM
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.
LGTM
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…of min/max width, and simplify overlay positioning.