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

first column of objects table fixed #3147

Merged

Conversation

Muralidhar22
Copy link
Contributor

@Muralidhar22 Muralidhar22 commented Dec 23, 2023

closes #2657

Before

Screenshot 2023-12-24 at 2 01 28 AM

After

Screenshot 2023-12-24 at 2 03 15 AM

@Muralidhar22 Muralidhar22 marked this pull request as draft December 23, 2023 14:52
Copy link

github-actions bot commented Dec 23, 2023

CLA

Hello there and welcome to our project!
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.
Although we don't have a dedicated legal counsel, having this kind of agreement can protect us from potential legal issues or patent trolls.
Thank you for your understanding.

Generated by 🚫 dangerJS against 0c2384a

@Muralidhar22 Muralidhar22 force-pushed the ui-fix/objects-table-first-column branch from 71477e7 to 6300a41 Compare December 23, 2023 20:28
@Muralidhar22 Muralidhar22 marked this pull request as ready for review December 23, 2023 20:34
Copy link
Contributor

@lucasbordeau lucasbordeau left a comment

Choose a reason for hiding this comment

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

@Muralidhar22 thanks for your contribution.

This issue requires a bit of extra work :

  • The table contains a lot of components and logic
  • Thus a full re-render is very costly and is visible by the user
  • We only manipulate CSS here
  • So we can use direct DOM manipulation via a tableRef

Please also try to create a separate component to hold this part of the tree :

<StyledTable ref={tableRef} className="entity-table-cell">
  <RecordTableHeader createRecord={createRecord} />
  <RecordTableBodyEffect />
  <RecordTableBody />
</StyledTable> 

Because the RecordTable component is handling too much logic.

You can get rid of isTabelScrolledState and use directly useInView in this new component, and toggle a .shadow class directly in the onChange event of useInView.

Copy link
Contributor

@lucasbordeau lucasbordeau left a comment

Choose a reason for hiding this comment

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

Nice, that's way better ! We can see it's instantly changing the CSS.

I did a bit of renaming and reorganizing.

@lucasbordeau lucasbordeau merged commit 2204345 into twentyhq:main Jan 2, 2024
6 checks passed
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.

The first column of objects table should be fixed
2 participants