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

Display table record creation row when clicking on Add new from table empty state #6174

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

charlesBochet
Copy link
Member

As per title

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

  • packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx: Added check for pendingRecordId to conditionally render RecordTableEmptyState.
  • packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx: Added handleAddButtonClick function to handle new record creation from the empty state.
  • packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx: Reordered import statements.

2 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)

  • packages/twenty-front/src/modules/object-record/record-table/components/RecordTable.tsx: Display table record creation row on 'Add new' click from empty state.
  • packages/twenty-front/src/modules/object-record/record-table/components/RecordTableContextProvider.tsx: Replaced useUpsertRecordV2 with useUpsertRecord.
  • packages/twenty-front/src/modules/object-record/record-table/components/RecordTableWithWrappers.tsx: Always display RecordTable component, removing conditional rendering of RecordTableEmptyState.
  • packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useUpsertRecord.ts: Refactored upsert logic to use Recoil callbacks and snapshot values.
  • packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useUpsertRecordV2.ts: Deleted file, consolidating upsert functionality.

5 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)

  • packages/twenty-front/jest.config.ts: Set 'silent' option to false for detailed logging during test runs.
  • packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__tests__/useUpsertRecord.test.tsx: Updated test cases to align with new useUpsertRecord hook signature requiring objectNameSingular parameter.
  • packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/__tests__/useUpsertRecord.test.tsx: Refactored tests for better maintainability by injecting state values more cleanly.

2 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)

  • packages/twenty-front/jest.config.ts: Set silent mode to true to suppress Jest's output logs during test runs.

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

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM

@charlesBochet charlesBochet merged commit ee7b6bf into main Jul 9, 2024
12 of 13 checks passed
@charlesBochet charlesBochet deleted the fix-create-empty-state branch July 9, 2024 12:58
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.

2 participants