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

✨ Improve data grid #3231

Merged
merged 11 commits into from
Jan 24, 2024
Merged

Conversation

magnh
Copy link
Contributor

@magnh magnh commented Jan 23, 2024

resolves #3234

What does this pull request change?

This pull request introduces the following improvements to the data-grid:

Most of these changes have been discussed with @yusijs.

  • ✨ Reexport @tanstack/react-table types to ease typing in apps using the data grid
  • 📌 Move eds-core-react to peer dependencies
    • This is neccessary because EDS uses React Context and the grid and the project should
      have the same React instance running. This makes it possible to set EDS Density of the
      table above the component.
  • ♻️ Move text truncating into default cell to enable overwriting cell content
    • This enables custom cells like popover, autocomplete or other cells that overflows the cell itself.
  • 🐛 Inherit row background color for pinned cells
    • This ensures hover color on the whole row when columns are pinned
  • 🐛 Support 100% width
    • Support string width and height
  • ✨ Allow setting minWidth of table
  • ✨ Expose getRowId callback from react-table
  • ✨ Expose virtualizer ref
    • This is needed to be able to run "scroll to" functionality in apps.
  • 🐛 Hide virtualizer rows top and bottom rows when not needed

Why is this pull request needed?

In order to implement this in our project, we need to be able to use custom cells, set 100% width and use the EdsProvider with the table. Also, there are some minor bugs discovered during implementation.

magnh added 7 commits January 18, 2024 15:48
This is neccessary because EDS uses React Context and the grid and the project should
have the same React instance running. This makes it possible to set EDS Density of the
table above the component.
…content

This enables custom cells like popover, autocomplete or other cells that overflows the cell itself.
This ensures hover color on the whole row
This is needed to be able to run "scroll to" functionality in apps.
@magnh magnh marked this pull request as ready for review January 23, 2024 07:51
@magnh magnh requested review from vnys and oddvernes as code owners January 23, 2024 07:51
@magnh
Copy link
Contributor Author

magnh commented Jan 23, 2024

I cannot edit reviewers, but I would like a review from @yusijs 😄

It seems like the checks are failing due to a missing secret, but I'm not sure whether it's on my side or yours?

@oddvernes oddvernes requested a review from yusijs January 23, 2024 08:12
@oddvernes
Copy link
Collaborator

I cannot edit reviewers, but I would like a review from @yusijs 😄

It seems like the checks are failing due to a missing secret, but I'm not sure whether it's on my side or yours?

I have added @yusijs as a reviewer 🙂
The checks failing from forked pull requests are on our end. We need to fix the github action, but in the meantime we can run the tests locally 👍

@magnh
Copy link
Contributor Author

magnh commented Jan 23, 2024

I have added @yusijs as a reviewer 🙂 The checks failing from forked pull requests are on our end. We need to fix the github action, but in the meantime we can run the tests locally 👍

Great, thank you!
The tests are running locally.

@oddvernes
Copy link
Collaborator

✅ Lint
✅ Tests
✅ Types

@magnh magnh requested a review from yusijs January 23, 2024 12:19
@magnh
Copy link
Contributor Author

magnh commented Jan 23, 2024

Lint, test and build is all ✅ on my side, but you might want to check again on you end @oddvernes after the last changes?

@oddvernes
Copy link
Collaborator

lint/test/types all seems to be in order still 👍

Copy link
Contributor

@yusijs yusijs 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
Collaborator

@oddvernes oddvernes left a comment

Choose a reason for hiding this comment

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

LGTM!

@magnh
Copy link
Contributor Author

magnh commented Jan 24, 2024

Screenshot 2024-01-24 at 09 44 25

@oddvernes
Copy link
Collaborator

Yes I will merge 👍 Was just trying the build downstream in an app for the final test but looks good

@oddvernes oddvernes merged commit 056bbe8 into equinor:develop Jan 24, 2024
4 of 6 checks passed
@magnh magnh deleted the chore/improve-eds-data-grid branch January 24, 2024 09:36
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.

✨ Data-grid improvements
3 participants