Fixes issue #6267: Hide Navigation Tabs if Single connected Account #6277
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Conditional display of
TabListinpackages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsCalendarChannelsContainer.tsx - Conditional display of
TabListinpackages/twenty-front/src/modules/settings/accounts/components/SettingsAccountsMessageChannelsContainer.tsx - Hides navigation tabs if only one connected account is present
- Improves UI by removing unnecessary elements when only one account is connected
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
There was a problem hiding this comment.
Hi @ViditJain123 thanks for the PR, I have left comments :)
| tabListId={SETTINGS_ACCOUNT_CALENDAR_CHANNELS_TAB_LIST_COMPONENT_ID} | ||
| tabs={tabs} | ||
| /> | ||
| <div style={{ display: hasMultipleAccounts ? 'block' : 'none' }}> |
There was a problem hiding this comment.
@ViditJain123 In React we should do something like {hasMultipleAccounts && <TabList ... />}
|
Also, we should not apply the additional padding in SettingsAccountsMessageChannelDetails in the case we are not in a tab. This results into an additional space. See by comparaison to Appearance page: vs 
I think we should extract this padding and place it in a StyledContainer in SettingsAccountsMessageChannelsContainer (resp. CalendarChannelsContainer) instead of having it in SettingsAccountsMessageChannelDetails. This way we could apply it in case of tabs and don't apply it in case of one connected account |
|
Actually @ViditJain123, I've just noticed that another PR was opened by @adithej and that they was assigned on the ticket. I'll prioritize their PR and close this one, sorry about that. Feel free to work on any non-assigned good first issue :) |
I wrapped the TabList with a div and have added conditional display property to make it visible only when we have move than one email address. This fixed the issue. I waiting for the review. :)
