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

Breadcrumb DropDown improvement #7546

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

ehconitin
Copy link
Contributor

@ehconitin ehconitin commented Oct 9, 2024

context - #7397 (review)
P.S. Apologies for the background music in the screen recording—I didn’t realize my mic was on while capturing it. 😅

2024-10-09.23-55-08.mov

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 enhances the user experience in the field creation process for the data model settings, focusing on improved navigation and field type handling.

  • Added field type handling in SettingsDataModelNewFieldBreadcrumbDropDown.tsx, disabling the 'Configure' step when no field type is selected
  • Implemented setting of field type in form context when selected in SettingsObjectNewFieldSelector.tsx
  • Modified navigation in SettingsObjectNewFieldConfigure.tsx to preserve field type when canceling configuration
  • Improved overall user feedback and prevented invalid navigation states throughout the field creation flow

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

const theme = useTheme();

const fieldType = searchParams.get('fieldType') as SettingsFieldType;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using a type guard or assertion function instead of type assertion

const handleClick = (isConfigureStep: boolean) => {
if (isConfigureStep) {
navigate(`/settings/objects/${objectSlug}/new-field/configure`);
const handleClick = (step: 'select' | 'configure') => {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: handleClick function could be simplified by using early returns

Copy link
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

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

Great! Thank you

@FelixMalfait FelixMalfait merged commit c57d8f1 into twentyhq:main Oct 9, 2024
6 of 7 checks passed
Copy link

github-actions bot commented Oct 9, 2024

Thanks @ehconitin for your contribution!
This marks your 44th 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 removeStep1-followup-fix branch October 10, 2024 05:56
harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
context -
twentyhq#7397 (review)
P.S. Apologies for the background music in the screen recording—I didn’t
realize my mic was on while capturing it. 😅


https://github.com/user-attachments/assets/0cd31aa7-9ce2-4577-a79a-73c9890f2905

---------

Co-authored-by: Félix Malfait <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants