Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Remove inline styling logic from DataGrid #6886

Closed
wants to merge 10 commits into from
Closed

feat: Remove inline styling logic from DataGrid #6886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b14c982
change DataGridRow.updateRowStyle to protected member from private pr…
KingOfTac Jan 8, 2024
5ea462a
Change files
KingOfTac Jan 8, 2024
d46d9bd
change file
KingOfTac Jan 8, 2024
7e45483
change method name and add docs
KingOfTac Jan 8, 2024
ea2bbb7
remove inline styling from data grid
KingOfTac Jan 9, 2024
974b717
update DataGrid API docs
KingOfTac Jan 9, 2024
968c4a7
change file
KingOfTac Jan 9, 2024
485cf6f
tweak story styles
KingOfTac Jan 9, 2024
bc12e53
Merge branch 'master' into users/kingoftac/foundation/update-grid-row…
KingOfTac Jan 9, 2024
b712527
Merge branch 'master' into users/kingoftac/foundation/update-grid-row…
scomea Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@microsoft-fast-foundation-ee614c70-ba01-4139-b580-bb76a115d096.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Remove inline styling logic from DataGrid and sub components in favor of consumer CSS implementations.",
"packageName": "@microsoft/fast-foundation",
"email": "[email protected]",
"dependentChangeType": "prerelease"
}
8 changes: 0 additions & 8 deletions packages/web-components/fast-foundation/docs/api-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,9 +1026,6 @@ export class FASTDataGrid extends FASTElement {
focusRowIndex: number;
static generateColumns(row: object): ColumnDefinition[];
generateHeader: GenerateHeaderOptions;
gridTemplateColumns: string;
// (undocumented)
protected gridTemplateColumnsChanged(): void;
// @internal (undocumented)
handleFocus(e: FocusEvent): void;
// @internal (undocumented)
Expand Down Expand Up @@ -1071,8 +1068,6 @@ export class FASTDataGridCell extends FASTElement {
disconnectedCallback(): void;
gridColumn: string;
// (undocumented)
protected gridColumnChanged(): void;
// (undocumented)
handleFocusin(e: FocusEvent): void;
// (undocumented)
handleFocusout(e: FocusEvent): void;
Expand All @@ -1099,9 +1094,6 @@ export class FASTDataGridRow extends FASTElement {
disconnectedCallback(): void;
// @internal (undocumented)
focusColumnIndex: number;
gridTemplateColumns: string;
// (undocumented)
protected gridTemplateColumnsChanged(): void;
// @internal (undocumented)
handleCellFocus(e: Event): void;
// @internal (undocumented)
Expand Down
35 changes: 14 additions & 21 deletions packages/web-components/fast-foundation/src/data-grid/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ export const myDataGrid = DataGrid.compose({

| Name | Privacy | Description | Parameters | Return | Inherited From |
| ------------------------- | --------- | ----------- | ------------------------------------------------------------------------ | ------ | -------------- |
| `gridColumnChanged` | protected | | | `void` | |
| `columnDefinitionChanged` | protected | | `oldValue: ColumnDefinition or null, newValue: ColumnDefinition or null` | `void` | |
| `handleFocusin` | public | | `e: FocusEvent` | `void` | |
| `handleFocusout` | public | | `e: FocusEvent` | `void` | |
Expand Down Expand Up @@ -166,7 +165,6 @@ export const myDataGrid = DataGrid.compose({

| Name | Privacy | Type | Default | Description | Inherited From |
| ------------------------ | ------- | ---------------------------- | ------- | --------------------------------------------------------------------------------------------------- | -------------- |
| `gridTemplateColumns` | public | `string` | | String that gets applied to the the css gridTemplateColumns attribute for the row | |
| `rowType` | public | `DataGridRowTypes` | | The type of row | |
| `rowData` | public | `object or null` | `null` | The base data for this row | |
| `columnDefinitions` | public | `ColumnDefinition[] or null` | `null` | The column definitions of the row | |
Expand All @@ -176,12 +174,11 @@ export const myDataGrid = DataGrid.compose({

#### Methods

| Name | Privacy | Description | Parameters | Return | Inherited From |
| ---------------------------- | --------- | --------------------------------------------- | --------------------------------------- | ------ | -------------- |
| `gridTemplateColumnsChanged` | protected | | | `void` | |
| `rowDataChanged` | protected | | | `void` | |
| `toggleSelected` | public | Attempts to set the selected state of the row | `detail: DataGridSelectionChangeDetail` | `void` | |
| `handleFocusout` | public | | `e: FocusEvent` | `void` | |
| Name | Privacy | Description | Parameters | Return | Inherited From |
| ---------------- | --------- | --------------------------------------------- | --------------------------------------- | ------ | -------------- |
| `rowDataChanged` | protected | | | `void` | |
| `toggleSelected` | public | Attempts to set the selected state of the row | `detail: DataGridSelectionChangeDetail` | `void` | |
| `handleFocusout` | public | | `e: FocusEvent` | `void` | |

#### Events

Expand All @@ -191,10 +188,9 @@ export const myDataGrid = DataGrid.compose({

#### Attributes

| Name | Field | Inherited From |
| ----------------------- | ------------------- | -------------- |
| `grid-template-columns` | gridTemplateColumns | |
| `row-type` | rowType | |
| Name | Field | Inherited From |
| ---------- | ------- | -------------- |
| `row-type` | rowType | |

#### Slots

Expand Down Expand Up @@ -236,7 +232,6 @@ export const myDataGrid = DataGrid.compose({
| ------------------------ | ------- | --------------------------------------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------- |
| `noTabbing` | public | `boolean` | `false` | When true the component will not add itself to the tab queue. Default is false. | |
| `generateHeader` | public | `GenerateHeaderOptions` | | Whether the grid should automatically generate a header row and its type | |
| `gridTemplateColumns` | public | `string` | | String that gets applied to the the css gridTemplateColumns attribute of child rows | |
| `pageSize` | public | `number or undefined` | | The number of rows to move selection on page up/down keystrokes. When undefined the grid will use viewport height/the height of the first non-header row. If the grid itself is a scrolling container it will be considered the viewport for this purpose, otherwise the document will be used. | |
| `selectionMode` | public | `DataGridSelectionMode` | | Defines how the grid handles row or cell selection. | |
| `selectionBehavior` | public | `DataGridSelectionBehavior` | | Controls selection behavior | |
Expand All @@ -254,21 +249,19 @@ export const myDataGrid = DataGrid.compose({

#### Methods

| Name | Privacy | Description | Parameters | Return | Inherited From |
| ---------------------------- | --------- | ----------- | ---------------- | ------ | -------------- |
| `noTabbingChanged` | protected | | | `void` | |
| `gridTemplateColumnsChanged` | protected | | | `void` | |
| `rowsDataChanged` | protected | | | `void` | |
| `columnDefinitionsChanged` | protected | | | `void` | |
| `handleRowSelectedChange` | public | | `e: CustomEvent` | `void` | |
| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------------------------- | --------- | ----------- | ---------------- | ------ | -------------- |
| `noTabbingChanged` | protected | | | `void` | |
| `rowsDataChanged` | protected | | | `void` | |
| `columnDefinitionsChanged` | protected | | | `void` | |
| `handleRowSelectedChange` | public | | `e: CustomEvent` | `void` | |

#### Attributes

| Name | Field | Inherited From |
| ----------------------- | ------------------- | -------------- |
| `no-tabbing` | noTabbing | |
| `generate-header` | generateHeader | |
| `grid-template-columns` | gridTemplateColumns | |
| `page-size` | pageSize | |
| `selection-mode` | selectionMode | |
| `selection-behavior` | selectionBehavior | |
Expand Down
16 changes: 0 additions & 16 deletions packages/web-components/fast-foundation/src/data-grid/data-grid-cell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,6 @@ export class FASTDataGridCell extends FASTElement {
*/
@attr({ attribute: "grid-column" })
public gridColumn: string;
protected gridColumnChanged(): void {
if (this.$fastController.isConnected) {
this.updateCellStyle();
}
}

/**
* The base data for the parent row
Expand Down Expand Up @@ -134,14 +129,7 @@ export class FASTDataGridCell extends FASTElement {
this.addEventListener(eventFocusOut, this.handleFocusout);
this.addEventListener(eventKeyDown, this.handleKeydown);

this.style.gridColumn = `${
this.columnDefinition?.gridColumn === undefined
? 0
: this.columnDefinition.gridColumn
}`;

this.updateCellView();
this.updateCellStyle();
}

/**
Expand Down Expand Up @@ -325,8 +313,4 @@ export class FASTDataGridCell extends FASTElement {
this.customCellView = null;
}
}

private updateCellStyle = (): void => {
this.style.gridColumn = this.gridColumn;
};
}
13 changes: 0 additions & 13 deletions packages/web-components/fast-foundation/src/data-grid/data-grid-row.pw.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,6 @@ test.describe("DataGridRow", () => {
await expect(element).toHaveAttribute("role", "row");
});

test("should set `grid-template-columns` style to match attribute", async () => {
await root.evaluate(node => {
node.innerHTML = /* html */ `
<fast-data-grid-row grid-template-columns="1fr 2fr 3fr"></fast-data-grid-row>
`;
});

await expect(element).toHaveAttribute(
"style",
"grid-template-columns: 1fr 2fr 3fr;"
);
});

test("should fire an event when a child cell is focused", async () => {
await root.evaluate(node => {
node.innerHTML = /* html */ `
Expand Down
21 changes: 0 additions & 21 deletions packages/web-components/fast-foundation/src/data-grid/data-grid-row.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,6 @@ import {
* @public
*/
export class FASTDataGridRow extends FASTElement {
/**
* String that gets applied to the the css gridTemplateColumns attribute for the row
*
* @public
* @remarks
* HTML Attribute: grid-template-columns
*/
@attr({ attribute: "grid-template-columns" })
public gridTemplateColumns: string;
protected gridTemplateColumnsChanged(): void {
if (this.$fastController.isConnected) {
this.updateRowStyle();
}
}

/**
* The type of row
*
Expand Down Expand Up @@ -211,8 +196,6 @@ export class FASTDataGridRow extends FASTElement {
this.addEventListener(eventKeyDown, this.handleKeydown);
this.addEventListener(eventClick, this.handleClick);

this.updateRowStyle();

if (this.refocusOnLoad) {
// if focus was on the row when data changed try to refocus on same cell
this.refocusOnLoad = false;
Expand Down Expand Up @@ -354,8 +337,4 @@ export class FASTDataGridRow extends FASTElement {
? this.headerCellItemTemplate
: this.defaultHeaderCellItemTemplate;
}

private updateRowStyle = (): void => {
this.style.gridTemplateColumns = this.gridTemplateColumns;
};
}
54 changes: 1 addition & 53 deletions packages/web-components/fast-foundation/src/data-grid/data-grid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,6 @@ export class FASTDataGrid extends FASTElement {
});
}

/**
* generates a gridTemplateColumns based on columndefinitions
*/
private static generateTemplateColumns(
columnDefinitions: ColumnDefinition[]
): string {
let templateColumns: string = "";
columnDefinitions.forEach((column: ColumnDefinition) => {
templateColumns = `${templateColumns}${
templateColumns === "" ? "" : " "
}${"1fr"}`;
});
return templateColumns;
}

/**
* Default callback to determine if a row is selectable (also depends on selectionMode)
* By default all rows except for header rows are selectable
Expand Down Expand Up @@ -198,21 +183,6 @@ export class FASTDataGrid extends FASTElement {
}
}

/**
* String that gets applied to the the css gridTemplateColumns attribute of child rows
*
* @public
* @remarks
* HTML Attribute: grid-template-columns
*/
@attr({ attribute: "grid-template-columns" })
public gridTemplateColumns: string;
protected gridTemplateColumnsChanged(): void {
if (this.$fastController.isConnected) {
this.updateRowIndexes();
}
}

/**
* The number of rows to move selection on page up/down keystrokes.
* When undefined the grid will use viewport height/the height of the first non-header row.
Expand Down Expand Up @@ -312,9 +282,7 @@ export class FASTDataGrid extends FASTElement {
if (!this.columnDefinitions) {
return;
}
this.generatedGridTemplateColumns = FASTDataGrid.generateTemplateColumns(
this.columnDefinitions
);

if (this.$fastController.isConnected) {
this.columnDefinitionsStale = true;
this.queueRowIndexUpdate();
Expand Down Expand Up @@ -457,8 +425,6 @@ export class FASTDataGrid extends FASTElement {
private rowindexUpdateQueued: boolean = false;
private columnDefinitionsStale: boolean = true;

private generatedGridTemplateColumns: string = "";

private lastNotShiftSelectedRowIndex = -1;
private preShiftRowSelection: number[] | null = null;

Expand Down Expand Up @@ -941,7 +907,6 @@ export class FASTDataGrid extends FASTElement {
);
this.generatedHeader = generatedHeaderElement as unknown as FASTDataGridRow;
this.generatedHeader.columnDefinitions = this.columnDefinitions;
this.generatedHeader.gridTemplateColumns = this.gridTemplateColumns;
this.generatedHeader.rowType =
this.generateHeader === GenerateHeaderOptions.sticky
? DataGridRowTypes.stickyHeader
Expand Down Expand Up @@ -987,26 +952,9 @@ export class FASTDataGrid extends FASTElement {
};

private updateRowIndexes = (): void => {
let newGridTemplateColumns = this.gridTemplateColumns;

if (newGridTemplateColumns === undefined) {
// try to generate columns based on manual rows
if (this.generatedGridTemplateColumns === "" && this.rowElements.length > 0) {
const firstRow: FASTDataGridRow = this.rowElements[0] as FASTDataGridRow;
this.generatedGridTemplateColumns = new Array(
firstRow.cellElements.length
)
.fill("1fr")
.join(" ");
}

newGridTemplateColumns = this.generatedGridTemplateColumns;
}

this.rowElements.forEach((element: Element, index: number): void => {
const thisRow = element as FASTDataGridRow;
thisRow.rowIndex = index;
thisRow.gridTemplateColumns = newGridTemplateColumns;
thisRow.selectionBehavior = this.selectionBehavior;
if (
this.selectionMode === DataGridSelectionMode.singleRow ||
Expand Down
1 change: 1 addition & 0 deletions packages/web-components/fast-foundation/src/data-grid/stories/data-grid-cell.register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { dataGridCellTemplate } from "../data-grid-cell.template.js";

const styles = css`
:host {
grid-column: ${x => x.gridColumn ?? 0};
color: var(--neutral-foreground-rest);
box-sizing: border-box;
padding: calc(var(--design-unit) * 1px) calc(var(--design-unit) * 3px);
Expand Down
4 changes: 4 additions & 0 deletions packages/web-components/fast-foundation/src/data-grid/stories/data-grid-row.register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { dataGridRowTemplate } from "../data-grid-row.template.js";
const styles = css`
:host {
display: grid;
grid-template-columns: subgrid;
grid-column-start: 1;
grid-column-end: span all;
grid-auto-flow: row;
padding: 1px 0;
box-sizing: border-box;
width: 100%;
Expand Down
2 changes: 0 additions & 2 deletions packages/web-components/fast-foundation/src/data-grid/stories/data-grid-row.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const storyTemplate = html<StoryArgs<FASTDataGridRow>>`
<fast-data-grid-row
:columnDefinitions="${x => x.columnDefinitions}"
:rowData="${x => x.rowData}"
grid-template-columns="${x => x.gridTemplateColumns}"
row-type="${x => x.rowType}"
>
${x => x.storyContent}
Expand All @@ -23,7 +22,6 @@ export default {
},
argTypes: {
columnDefinitions: { control: "object" },
gridTemplateColumns: { control: "text" },
rowType: { control: "select", options: Object.values(DataGridRowTypes) },
storyContent: { table: { disable: true } },
},
Expand Down
7 changes: 6 additions & 1 deletion packages/web-components/fast-foundation/src/data-grid/stories/data-grid.register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import { registerComplexCell } from "./examples/complex-cell.js";

const styles = css`
:host {
display: block;
display: grid;
grid-auto-flow: row;
grid-template-columns: repeat(
${x => x.columnDefinitions?.length ?? 1},
minmax(0, 1fr)
);
}

:host([selection-mode="multi-row"]) {
Expand Down
Loading
Loading