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: Nav Item Api & Webhook and Functions stay selected #7628

Conversation

tushar110302
Copy link
Contributor

Fixed Nav Item Api & Webhook and Functions stay selected like data model settings.
Now when clicked stays selected and deos not loose its selection

Fixes #7573

Screen.Recording.2024-10-13.at.9.43.25.AM.mp4

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 modifies the SettingsNavigationDrawerItems component to fix the selection behavior of 'API & Webhooks' and 'Functions' navigation items in the settings menu.

  • Added matchSubPages prop to 'API & Webhooks' and 'Functions' items in packages/twenty-front/src/modules/settings/components/SettingsNavigationDrawerItems.tsx
  • Ensures these items remain selected when navigating to sub-pages, similar to data model settings
  • Addresses issue Settings Nav item doesn't stay selected  #7573 where 'API & Webhooks' menu item lost selection when accessing webhook pages
  • Improves user experience by maintaining consistent navigation state across settings sub-sections

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

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.

LGTM! @tushar110302 Thank you for contributing!

@tushar110302
Copy link
Contributor Author

LGTM! @tushar110302 Thank you for contributing!

Happy to contribute and become a part of it 🙂

@FelixMalfait
Copy link
Member

Thanks @tushar110302!
@ehconitin as discussed this works well but I think we should consider inverting the API so that people who add sub-pages to a page that didn't have any don't have to think about changing this setting. Currently the only page where we want to disable this is accounts where we display calendar/email directly

@FelixMalfait FelixMalfait merged commit ccdef0e into twentyhq:main Oct 13, 2024
8 checks passed
Copy link

oss-gg bot commented Oct 13, 2024

Awarding tushar110302: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/tushar110302

@ehconitin ehconitin mentioned this pull request Oct 13, 2024
FelixMalfait pushed a commit that referenced this pull request Oct 13, 2024
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.

Settings Nav item doesn't stay selected
3 participants