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

fix: object activate dropdown #8438

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

harshrajeevsingh
Copy link
Contributor

@harshrajeevsingh harshrajeevsingh commented Nov 8, 2024

Fixes: #8436
Fixes: #8435
& other duplicate DropdownMenu

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

Simplified the dropdown menu structure in Settings Object Inactive Menu by removing redundant DropdownMenu wrapper while maintaining core functionality.

  • Removed unnecessary DropdownMenu wrapper in /packages/twenty-front/src/modules/settings/data-model/objects/components/SettingsObjectInactiveMenuDropDown.tsx to fix activation button bug
  • Preserved conditional rendering of delete option for custom objects
  • Maintained proper dropdown scope and hotkey handling

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@ehconitin
Copy link
Contributor

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=34855-566375&node-type=instance&t=Qd41V7OJhUGr6u0B-0

Screenshot 2024-11-08 at 22 51 41

@harshrajeevsingh we should match width to figma, thanks ;)

@Bonapara its called erase and not delete in figma :)

Copy link
Contributor

@ehconitin ehconitin left a comment

Choose a reason for hiding this comment

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

looks good --- just need to adjust width

@harshrajeevsingh
Copy link
Contributor Author

harshrajeevsingh commented Nov 8, 2024

I found the same issue for Object's Field Disabled Dropdown. Working on it.

Screenshot from 2024-11-08 23-48-55

@FelixMalfait
Copy link
Member

Good catch thanks! Same as here: https://github.com/twentyhq/twenty/pull/8374/files

Would you wind also correcting it here?
Screenshot 2024-11-09 at 08 21 38

(need to un hide this in your .env)

# SUPPORT_DRIVER=front
# SUPPORT_FRONT_HMAC_KEY=replace_me_with_front_chat_verification_secret
# SUPPORT_FRONT_CHAT_ID=replace_me_with_front_chat_id

@harshrajeevsingh
Copy link
Contributor Author

@FelixMalfait There are even more components with DropdownMenu duplicate.

Screenshot from 2024-11-09 15-19-59

Screenshot from 2024-11-09 15-32-50

A few of them can't be seen because they are the same width.
Also, how can I find MessageThreadSubscribersDropdownButton in UI? I know it's related to Emails, but I can't find it

@ehconitin
Copy link
Contributor

I will take care of MessageThreadSubscribersDropdownButton

@harshrajeevsingh
Copy link
Contributor Author

@ehconitin All set on my end!
This PR also resolves an additional issue, which I’ve detailed in the description. Thanks for your guidance! :)

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.

Perfect thank you!

@FelixMalfait FelixMalfait merged commit 6d62dd9 into twentyhq:main Nov 11, 2024
16 checks passed
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.

Activate object button is bugged Missing aria-label for inactive field button
3 participants