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: conditionally rendered Tablist only if there are multiple accounts. #6274

Merged
merged 5 commits into from
Jul 16, 2024

Conversation

adithej
Copy link
Contributor

@adithej adithej commented Jul 15, 2024

issue - 6267

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

  • Conditionally render TabList in SettingsAccountsCalendarChannelsContainer.tsx if multiple calendar channels exist (packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarChannelsContainer.tsx).
  • Conditionally render TabList in SettingsAccountsMessageChannelsContainer.tsx if multiple accounts exist (packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsMessageChannelsContainer.tsx).

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

@charlesBochet
Copy link
Member

Hi @adithej, thanks for your contribution! The code looks good but we have a padding issue, see comment on another (duplicated) PR here: #6277

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

(updates since last review)

  • Added CalendarEventDetailsEffect component to handle upsertion of calendar event records (packages/twenty-front/src/modules/activities/calendar/components/CalendarEventDetailsEffect.tsx)
  • Included CalendarEventDetailsEffect and RecordValueSetterEffect in RightDrawerCalendarEvent.tsx (packages/twenty-front/src/modules/activities/calendar/right-drawer/components/RightDrawerCalendarEvent.tsx)
  • Removed exclusion of 'probability' fields in filter definitions (packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemsAsFilterDefinitions.ts)
  • Introduced Recoil state for dynamic mutation page size in useDeleteManyRecords (packages/twenty-front/src/modules/object-record/hooks/useDeleteManyRecords.ts)
  • Removed FieldMetadataType.PROBABILITY from various files, impacting field handling and validation (packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.entity.ts, packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/validate-default-value-for-type.util.ts)

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

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

(updates since last review)

  • Conditionally render TabList in SettingsAccountsCalendarChannelsContainer.tsx if multiple calendar channels (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarChannelsContainer.tsx)
  • Conditionally render TabList in SettingsAccountsMessageChannelsContainer.tsx if multiple accounts (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsMessageChannelsContainer.tsx)
  • Remove padding-top from StyledDetailsContainer in SettingsAccountsCalendarChannelDetails.tsx (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarChannelDetails.tsx)
  • Remove padding-top from StyledDetailsContainer in SettingsAccountsMessageChannelDetails.tsx (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsMessageChannelDetails.tsx)
  • Introduce StyledMessageContainer for bottom padding in SettingsAccountsMessageChannelsContainer.tsx (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsMessageChannelsContainer.tsx)

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

@adithej
Copy link
Contributor Author

adithej commented Jul 16, 2024

Hi @charlesBochet, I noticed the conversation regarding the padding issue. I have rectified it , let me know it need any more changes

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

LGTM!

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

(updates since last review)

  • Conditionally render TabList in SettingsAccountsCalendarChannelsContainer.tsx if multiple calendar channels (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarChannelsContainer.tsx)
  • Conditionally render TabList in SettingsAccountsMessageChannelsContainer.tsx if multiple accounts (/packages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsMessageChannelsContainer.tsx)

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

@charlesBochet charlesBochet merged commit a8dfff3 into twentyhq:main Jul 16, 2024
11 checks passed
Copy link

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

Contributions

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