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.

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

Kanban card creation revamp #7169

Merged
merged 13 commits into from
Sep 25, 2024
Merged

Conversation

ehconitin
Copy link
Contributor

@ehconitin ehconitin commented Sep 20, 2024

fixes #6957

@ehconitin
Copy link
Contributor Author

@Bonapara work is still in progress but let me know your thoughts from product perspective.
Thanks

2024-09-20.10-43-09.mp4

@Bonapara
Copy link
Member

Hi @ehconitin, thanks! Could you use the floating input instead of placing the input within the card?

image

@ehconitin
Copy link
Contributor Author

@Bonapara will do!
@Bonapara Question -
we do not have to set name for opportunity kanban right? it will still take the company/relations name?
@lucasbordeau could you let me know if this approach is correct?

2024-09-20.14-43-36.mp4

@Bonapara
Copy link
Member

@ehconitin you're right. When creating an opportunity, the card name should be the company name. This only applies to cards on other kanbans where users must set a record name manually.

@ehconitin
Copy link
Contributor Author

2024-09-20.19-44-03.mp4

@Bonapara
Copy link
Member

Hi @ehconitin, thanks! Could you use the floating input instead of placing the input within the card?

image

Great! Can you apply this design? I was also wondering if we can make the stage appear on the card before the title is set and the record saved.

@ehconitin
Copy link
Contributor Author

ehconitin commented Sep 20, 2024

@Bonapara
could u reshare the image, it seems to be broken on my side. refresh isnt helping

Ah nvm got it.

@ehconitin
Copy link
Contributor Author

Hi @ehconitin, thanks! Could you use the floating input instead of placing the input within the card?
image

Great! Can you apply this design? I was also wondering if we can make the stage appear on the card before the title is set and the record saved.

Yup, on it!

@ehconitin
Copy link
Contributor Author

ehconitin commented Sep 20, 2024

@Bonapara
I think having stage appear before creating a card will have many challenges, but let me ask @lucasbordeau if he could think of an easier way!
Floating input is done btw!

2024-09-20.23-29-41.mp4

@ehconitin ehconitin marked this pull request as ready for review September 21, 2024 17:10
@ehconitin
Copy link
Contributor Author

Should I address #7002 in this PR in a similar manner, or handle it separately? Let me know what works best!

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request implements a new feature for creating records directly on the Kanban board, addressing issue #6957. Here's a summary of the key changes:

  • Added a new text input for creating records in the RecordBoardCard component
  • Updated RecordBoardColumnCardsContainer, RecordBoardColumnHeader, and RecordBoardColumnNewButton to support the new card creation functionality
  • Introduced new state management with recordBoardNewRecordByColumnIdComponentFamilyState and recordBoardNewRecordByColumnIdSelector
  • Modified the useAddNewCard hook to handle creating a new card with a title and position
  • Updated RecordIndexPageKanbanAddButton to support creating a record directly on the Kanban board without navigating to the record detail page

These changes align with the desired behavior described in the issue, allowing users to set a record name when creating a new card on the Kanban board, similar to the table view functionality.

8 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings

Comment on lines 230 to 236
const handleCreate = (text: string) => {
if (text.trim() !== '' && position !== undefined) {
handleAddNewCardClick(text.trim(), position);
setNewRecordName('');
onCreateSuccess?.();
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Add error handling for failed card creation

@ehconitin
Copy link
Contributor Author

Label identifier issue fixed!

2024-09-24.17-03-50.mp4

@bosiraphael bosiraphael self-assigned this Sep 24, 2024
@Bonapara
Copy link
Member

There is not floating input yet @ehconitin right?

@ehconitin
Copy link
Contributor Author

@Bonapara there was a floating input already but styles like box shadow were missing, fixed it!

@ehconitin
Copy link
Contributor Author

@Bonapara
#7163

All inline field heights should be 24px, as they are on record pages.

There should be a 2px gap between inline field rows.

image

Comment on lines 33 to 69
const { visibleFieldDefinitionsState } = useRecordBoardStates();

const visibleFieldDefinitions = useRecoilValue(
visibleFieldDefinitionsState(),
);
const labelIdentifierField = visibleFieldDefinitions.find(
(field) => field.isLabelIdentifier,
);

const newRecord = useRecoilValue(
recordBoardNewRecordByColumnIdSelector({
familyKey: columnId,
scopeId: columnId,
}),
);

const { handleAddNewCardClick, handleCreateSuccess } = useAddNewCard();

const handleNewButtonClick = () => {
handleAddNewCardClick(
labelIdentifierField?.label ?? '',
'',
'last',
columnId,
);
};

if (newRecord.isCreating && newRecord.position === 'last') {
return (
<RecordBoardCard
isCreating={true}
onCreateSuccess={() => handleCreateSuccess('last')}
position="last"
/>
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like duplicate code with packages/twenty-front/src/modules/object-record/record-board/record-board-column/components/RecordBoardColumnHeader.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will refactor them!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bosiraphael Done! I had to create a new hook due to a scoping issue, as it couldn't be kept within useAddNewCard

Copy link
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

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

LGTM :)

@bosiraphael bosiraphael merged commit 7752510 into twentyhq:main Sep 25, 2024
5 of 11 checks passed
Copy link

Thanks @ehconitin for your contribution!
This marks your 36th PR on the repo. You're top 1% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

@ehconitin ehconitin deleted the KanbanCardCreation branch October 10, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set a record name on Kanban card creation
3 participants