-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Build empty state for remote tables #5652
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
- Introduced an empty state placeholder for remote tables in
RecordTable.tsx
- Added
recordTableId
parameter touseLoadRecordIndexTable.ts
anduseRecordTableRecordGqlFields.ts
- Modified
RecordTableBody.tsx
to accepttableRowIds
as a prop - Added
RecordTableEmptyState.tsx
component for user-friendly empty state messages
Icon={IconSettings} | ||
title="Go to Settings" | ||
variant={'secondary'} | ||
onClick={() => () => navigate('/settings/integrations')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the extra arrow function to avoid unnecessary nesting: onClick={() => navigate('/settings/integrations')}
.
bc6ca62
to
5f886e3
Compare
const { tableRowIdsState } = useRecordTableStates(recordTableId); | ||
const tableRowIds = useRecoilValue(tableRowIdsState); | ||
|
||
const { loading } = useLoadRecordIndexTable( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RecordIndex is the parent page which loading either the RecordTable or the RecordBoard. We should not call a recordIndex hook within the table, the dependency is in the wrong way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure to understand the changes here:
- why pass tableRowIds to RecordTableBody instead of using recoil state?
- why drilling recordTableId? wasn't it working before
The dependency is also in the wrong way between recordIndexTable and RecordTable (it's actually kind of anti-pattern in the code I remember)
5f886e3
to
9464b1d
Compare
isRemote: boolean; | ||
}; | ||
|
||
export const RecordTableEmptyState = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add stories here!
541e42b
to
ca2e6e9
Compare
Remote tables could be in an empty state because: - either we do not have data, which is normal - either the connexion is broken (issue with the server, table requires updates...) Apollo throws errors but these will quickly disappear and do not provide any tips to the user on how handle those. This PR adds a new empty state placeholder for remote objects, that will be display when the record list is empty. It will provide a link to the settings page. <img width="1512" alt="Capture d’écran 2024-05-30 à 11 49 33" src="https://github.com/twentyhq/twenty/assets/22936103/fc2dd3cc-e90b-4033-b023-83ac9ff2a70b">
Remote tables could be in an empty state because: - either we do not have data, which is normal - either the connexion is broken (issue with the server, table requires updates...) Apollo throws errors but these will quickly disappear and do not provide any tips to the user on how handle those. This PR adds a new empty state placeholder for remote objects, that will be display when the record list is empty. It will provide a link to the settings page. <img width="1512" alt="Capture d’écran 2024-05-30 à 11 49 33" src="https://github.com/twentyhq/twenty/assets/22936103/fc2dd3cc-e90b-4033-b023-83ac9ff2a70b">
Remote tables could be in an empty state because:
Apollo throws errors but these will quickly disappear and do not provide any tips to the user on how handle those.
This PR adds a new empty state placeholder for remote objects, that will be display when the record list is empty. It will provide a link to the settings page.