Splits context menu into separate component #6788
Conversation
There was a problem hiding this comment.
copied from data_table_component.ng.html
There was a problem hiding this comment.
Copied from data_table_component.scss
There was a problem hiding this comment.
Copied from data_table_component.ts
There was a problem hiding this comment.
Mostly copied from data_table_test.ts with some tests changed to be parametrized
| @@ -104,7 +56,7 @@ | |||
| <button | |||
| mat-icon-button | |||
| class="add-column-btn" | |||
| (click)="openColumnSelector($event)" | |||
| (click)="openColumnSelector({event: $event})" | |||
There was a problem hiding this comment.
To allow ContextMenuComponent to propagate openColumnSelector parameters through its output, we need to make openColumnSelector's parameters an object as an Event can only be an object.
| @@ -363,8 +328,8 @@ export class DataTableComponent implements OnDestroy, AfterContentInit { | |||
| }); | |||
| } | |||
|
|
|||
| openFilterMenu(event: MouseEvent, header: ColumnHeader) { | |||
There was a problem hiding this comment.
header is exactly equivalent to this.contextMenuHeader
| @@ -17,6 +17,7 @@ limitations under the License. | |||
| :host { | |||
| display: flex; | |||
| font-size: 13px; | |||
| min-width: 210px; | |||
There was a problem hiding this comment.
Fixes discrete filter placeholder text from being cut off:
|
|
||
| return index; | ||
| } | ||
|
|
||
| onColumnAdded(header: ColumnHeader) { |
There was a problem hiding this comment.
I played with your changes. "Insert Column Left" no longer works as expected. It inserts to the right instead.
There was a problem hiding this comment.
Oh thanks for catching this! To clarify, the quoted "getInsertIndex()" code should be irrelevant as it's unused, but the other changes introduced a race condition where insertColumnTo is reset to undefined after it is first properly set on column open.
Added a temporary fix (namely don't reset insertColumnTo to undefined - this is actually unnecessary) and included a simple regression test. Note that this class of problem should be irrelevant in the next PR that creates modals dynamically.
## Motivation for features / changes Splits DataTableComponent's context menu functionality into its own component to be compatible with future updates that will enable dynamic custom modal creation (for alleviating [stacking context issues](tensorflow#6737 (comment))) ## Technical description of changes - No new functionality - simply moves the context menu related template and logic to a new ContextMenuComponent - Parameterizes some context menu tests for legibility. ## Detailed steps to verify changes work correctly (as executed by you) - Tested manually that all context menu features work as before
Motivation for features / changes
Splits DataTableComponent's context menu functionality into its own component to be compatible with future updates that will enable dynamic custom modal creation (for alleviating stacking context issues)
Technical description of changes
Detailed steps to verify changes work correctly (as executed by you)