-
Notifications
You must be signed in to change notification settings - Fork 161
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
IgxTreeGrid - Editing #2909
IgxTreeGrid - Editing #2909
Conversation
# Conflicts: # projects/igniteui-angular/src/lib/grids/api.service.ts
…I/igniteui-angular into VSlavov/treeGrid-editing
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-expanding.spec.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-expanding.spec.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-expanding.spec.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-integration.spec.ts
Show resolved
Hide resolved
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-integration.spec.ts
Show resolved
Hide resolved
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-integration.spec.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid-integration.spec.ts
Outdated
Show resolved
Hide resolved
…eGrid-editing # Conflicts: # projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.ts
%grid-cell-text { | ||
font-style: italic; | ||
color: igx-color(map-get($theme, 'palette'), 'error'); | ||
text-decoration-line: line-through; |
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.
@simeonoff Can't this be just text-decoration
so it'd work in IE/Edge too?
@StefanIvanov Is red part of the design?
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.
@damyanpetev text-decoration-line is just a property of the shorthand text-decoration. The line-through
text decoration is not supported in IE and Edge. As to the color being red, I've discussed it with @StefanIvanov before implementing the change.
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.
@simeonoff Despite being the shorthand, text-decoration: line-through;
is in the CSS1 spec and works just fine even in older IE-s. So unless we need any of the decorations, can we use the shorthand for consistency?
…eGrid-editing # Conflicts: # projects/igniteui-angular/src/lib/grids/api.service.ts # projects/igniteui-angular/src/lib/grids/grid-base.component.ts
@@ -4385,7 +4391,14 @@ export abstract class IgxGridBaseComponent implements OnInit, OnDestroy, AfterCo | |||
public repositionRowEditingOverlay(row: IgxRowComponent<IgxGridBaseComponent>) { | |||
this.configureRowEditingOverlay(row.rowID); | |||
if (!this.rowEditingOverlay.collapsed) { | |||
this.rowEditingOverlay.reposition(); | |||
const rowStyle = this.rowEditingOverlay.element.parentElement.style; | |||
if (row) { |
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.
There's something not quite right here - if the method accepts a call without a row, the .configureRowEditingOverlay(row.rowID)
right above doesn't and will error out (see tests). Also what's with .configureRowEditingOverlay(row)
below - is it row or rowID?
…I/igniteui-angular into VSlavov/treeGrid-editing
Closes #2908.