Add no value column on Kanban #6252
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Added 'no value' column to Kanban board (
packages/twenty-front/src/modules/object-record/utils/computeRecordBoardColumnDefinitionsFromObjectMetadata.ts) - Introduced new column types for Kanban (
packages/twenty-front/src/modules/object-record/record-board/types/RecordBoardColumnDefinition.ts) - Conditional color setting for 'no value' column (
packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnHeader.tsx) - Reordered import statements for consistency (
packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx)
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added variant prop to
Tagcomponent for 'no value' column (packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnHeader.tsx) - Updated
boardFieldSelectValueprop to accept null values (packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardColumnLoaderEffect.tsx) - Added new column for records with no value in Kanban board (
packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexBoardDataLoader.tsx) - Handled null
columnFieldSelectValuein filter for 'no value' column (packages/twenty-front/src/modules/object-record/record-index/hooks/useLoadRecordIndexBoardColumn.ts) - Reordered import statements in
Tagcomponent (packages/twenty-ui/src/display/tag/components/Tag.tsx)
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
| boardFieldMetadataId={recordIndexKanbanFieldMetadataId} | ||
| boardFieldSelectValue={null} | ||
| recordBoardId={recordBoardId} | ||
| columnId={'no-value'} |
There was a problem hiding this comment.
Style: Consider using a constant for 'no-value' to avoid magic strings.
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Conditional rendering for
columnDefinition.actionsinRecordBoardColumnHeader.tsx - Display header actions and dropdown menu only when actions are available
- Relevant for the new 'no value' column in the Kanban board
No major changes found since last review.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Stricter check for
columnDefinition.actionslength inpackages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnHeader.tsx - Render actions only if
columnDefinition.actionshas items - Prevent potential rendering issues with empty
columnDefinition.actions
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
| @@ -79,14 +80,23 @@ export const RecordBoardColumnHeader = () => { | |||
| > | |||
| <Tag | |||
| onClick={handleBoardColumnMenuOpen} | |||
| color={columnDefinition.color} | |||
| variant={ | |||
| columnDefinition.type === RecordBoardColumnDefinitionType.Value | |||
There was a problem hiding this comment.
this is because of the Tag component not maching figma. we could have the transparent as a color variant
| @@ -45,6 +45,13 @@ export const RecordIndexBoardDataLoader = ({ | |||
| columnId={columnIds[index]} | |||
| /> | |||
| ))} | |||
| <RecordIndexBoardColumnLoaderEffect | |||
There was a problem hiding this comment.
I actually think this should be computed from the recordBoardcolumnDefinition array just to be consistent
|
Great! Maybe display @WorkspaceIsNullable() as "advanced option" during field creation/edition @Bonapara - could be a followup ticket? But the issue @charlesBochet is the data migration will be painful if we need to go back the other way and make the stage field not-nullable, which we will want to do. |
|
Okay, we have discussed this with @ijreilly and we are:
|
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added
isNullableproperty to field definition metadata inpackages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemAsFieldDefinition.ts - Conditional 'No value' option in
SelectFieldInput.tsxbased onisNullableproperty - Updated
FieldSelectMetadatatype to includeisNullableinFieldMetadata.ts - Conditional rendering of 'no-value' column in
RecordIndexBoardDataLoader.tsxif field is nullable - Added 'No Value' column to Kanban board in
computeRecordBoardColumnDefinitionsFromObjectMetadata.tsbased onisNullableproperty
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
| boardFieldMetadataId={recordIndexKanbanFieldMetadataId} | ||
| boardFieldSelectValue={null} | ||
| recordBoardId={recordBoardId} | ||
| columnId={'no-value'} |
There was a problem hiding this comment.
Style: Consider using a constant for 'no-value' to avoid magic strings.
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added
isNullableproperty toselectFieldDefinitioninpackages/twenty-front/src/modules/object-record/record-field/__mocks__/fieldDefinitions.ts - Added
PrefetchDataProviderto providers inpackages/twenty-front/src/testing/decorators/PageDecorator.tsx - Added new GraphQL query mock for
CombinedFindManyRecordsinpackages/twenty-front/src/testing/graphqlMocks.ts - Updated mock data for views and view fields in
packages/twenty-front/src/testing/mock-data/view-fields.tsandpackages/twenty-front/src/testing/mock-data/views.ts - Removed
variantprop fromTagcomponent inpackages/twenty-ui/src/display/tag/components/Tag.tsx
9 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
| console.log(columnDefinitions); | ||
| console.log(viewFields); |
There was a problem hiding this comment.
Style: Remove console.log statements before merging to main to avoid cluttering the console output.
| console.log(viewFields); | ||
|
|
There was a problem hiding this comment.
Style: Remove console.log statements before merging to main to avoid cluttering the console output.
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Removed
console.logstatements frompackages/twenty-front/src/modules/views/utils/mapViewFieldsToColumnDefinitions.ts
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Introduced
TagVarianttype with 'solid' and 'outline' values in/packages/twenty-ui/src/display/tag/components/Tag.tsx - Updated
TagPropsinterface to includevariantproperty - Modified
StyledTagto handle 'outline' variant with a dashed border
No major changes found since last review.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Open questions:
Follow up: