Skip to content

fix(editor): add default value to progress column #9637

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

Closed
wants to merge 1 commit into from
Closed

fix(editor): add default value to progress column #9637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 20 additions & 3 deletions blocksuite/affine/block-database/src/data-source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,25 @@ export class DatabaseBlockDataSource extends DataSourceBase {

propertyAdd(insertToPosition: InsertToPosition, type?: string): string {
this.doc.captureSync();

const metaConfig = this.propertyMetaGet(
type ?? propertyPresets.multiSelectPropertyConfig.type
);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

propertyMetaGet method looks like this

  propertyMetaGet(type: string): PropertyMetaConfig {
    return databaseBlockAllPropertyMap[type];
  }

It returns the property configuration object corresponding the type.


const result = addProperty(
this._model,
insertToPosition,
databaseBlockAllPropertyMap[
type ?? propertyPresets.multiSelectPropertyConfig.type
].create(this.newPropertyName())
metaConfig.create(this.newPropertyName())
);

updateCells(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause a large number of update records to exist in the CRDT. It is recommended not to do this. Instead, a default value should be returned when obtaining the value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause a large number of update records to exist in the CRDT. It is recommended not to do this. Instead, a default value should be returned when obtaining the value.

Thank you for super fast comment! @zzj3720

I'm not sure of how to implement returning a default value when obtaining the value. Could you please provide more guidance or an example to assist me in resolving this issue? I would greatly appreciate it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

/blocksuite/affine/data-view/src/property-presets/progress/define.ts
@@ -8,16 +8,16 @@ export const progressPropertyModelConfig =
     name: 'Progress',
     type: () => t.number.instance(),
     defaultData: () => ({}),
-    cellToString: ({ value }) => value?.toString() ?? '',
+    cellToString: ({ value }) => value?.toString() ?? '0',
     cellFromString: ({ value }) => {
-      const num = value ? Number(value) : NaN;
+      const num = value ? Number(value) : 0;
       return {
-        value: isNaN(num) ? null : num,
+        value: isNaN(num) ? 0 : num,
       };
     },
-    cellToJson: ({ value }) => value ?? null,
+    cellToJson: ({ value }) => value ?? 0,
     cellFromJson: ({ value }) => {
-      if (typeof value !== 'number') return undefined;
+      if (typeof value !== 'number') return 0;
       return value;
     },
     isEmpty: () => false,

How about changing all null or undefined values to return 0 like this? Although this change will internally store as null, it will work correctly when filtering. @zzj3720

this._model,
result,
Object.fromEntries(
this.rows$.value.map(r => [r, metaConfig.config.defaultValue])
)
);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it updates cells to default value whenever the property is added


applyPropertyUpdate(this._model);
return result;
}
Expand Down Expand Up @@ -306,8 +318,13 @@ export class DatabaseBlockDataSource extends DataSourceBase {
type: toType,
data: result.property,
}));
const metaConfig = this.propertyMetaGet(toType);
const cells: Record<string, unknown> = {};
currentCells.forEach((value, i) => {
if (value == null && result.cells[i] == null) {
cells[rows[i]] = metaConfig.config.defaultValue;
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it updates cells to default value whenever the property is updated to other type.


if (value != null || result.cells[i] != null) {
cells[rows[i]] = result.cells[i];
}
Expand Down
1 change: 1 addition & 0 deletions blocksuite/affine/data-view/src/core/property/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export type PropertyConfig<
newValue: Value;
}>
) => Value;
defaultValue?: Value;
};

export type DVJSON =
Expand Down
1 change: 1 addition & 0 deletions blocksuite/affine/data-view/src/property-presets/progress/define.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ export const progressPropertyModelConfig =
return value;
},
isEmpty: () => false,
defaultValue: 0,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The progress value always ranges from 0 to 100. When a new progress item is created, it is initially set to 0.

});
8 changes: 8 additions & 0 deletions blocksuite/affine/data-view/src/view-presets/table/table-view-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,14 @@ export class TableSingleView extends SingleViewBase<TableViewData> {
): string {
const id = super.rowAdd(insertPosition);

this.properties$.value.forEach(column =>
this.cellValueSet(
id,
column.id,
this.propertyMetaGet(column.type$.value).config.defaultValue
)
);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It initialize the cells to default value when a row is added


const filter = this.filter$.value;
if (filter.conditions.length > 0) {
const defaultValues = generateDefaultValues(filter, this.vars$.value);
Expand Down
Loading