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

DataGrid: load more and virtual scrolling #2799

Merged
merged 23 commits into from
Oct 17, 2023
Merged

DataGrid: load more and virtual scrolling #2799

merged 23 commits into from
Oct 17, 2023

Conversation

wanghoppe
Copy link
Member

No description provided.

@wanghoppe wanghoppe changed the title DataGrid: load more and virtual scrolling DataGrid: load more and virtual scrolling AB#25124895 Sep 12, 2023
@wanghoppe wanghoppe changed the title DataGrid: load more and virtual scrolling AB#25124895 DataGrid: load more and virtual scrolling Sep 12, 2023
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #2799 (f711ad1) into main (edd7a35) will decrease coverage by 0.04%.
The diff coverage is 63.06%.

❗ Current head f711ad1 differs from pull request most recent head 139ceb2. Consider uploading reports for the commit 139ceb2 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2799      +/-   ##
==========================================
- Coverage   66.54%   66.51%   -0.04%     
==========================================
  Files        1240     1245       +5     
  Lines       33961    34115     +154     
  Branches     6149     6219      +70     
==========================================
+ Hits        22600    22692      +92     
- Misses      11226    11286      +60     
- Partials      135      137       +2     
Files Coverage Δ
packages/bonito-core/src/errors.ts 100.00% <100.00%> (ø)
...ckages/bonito-core/src/util/cancellable-promise.ts 100.00% <100.00%> (ø)
packages/bonito-core/src/util/index.ts 100.00% <100.00%> (ø)
...ckages/bonito-ui/src/components/data-grid/index.ts 100.00% <100.00%> (ø)
packages/bonito-ui/src/hooks/index.ts 100.00% <100.00%> (ø)
packages/bonito-ui/src/theme/explorer-theme.ts 100.00% <ø> (ø)
packages/bonito-ui/src/theme/theme-util.ts 26.98% <ø> (ø)
packages/playground/src/layout/demo-nav-menu.tsx 100.00% <ø> (ø)
packages/bonito-ui/src/hooks/use-load-more.ts 97.14% <97.14%> (ø)
packages/playground/src/demo-routes.tsx 56.41% <50.00%> (-0.35%) ⬇️
... and 3 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edd7a35...139ceb2. Read the comment docs.

Copy link
Member

@dpwatrous dpwatrous left a comment

Choose a reason for hiding this comment

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

This is looking really good - I love the tests and demos you added. Let's follow up tomorrow and talk more about the overall design.

Random thought: this seems like a really good use case for some automated performance tests. I'd love to be able to track the performance of the data grid in various scenarios over time. We already have a few tests in Playwright which run a real Electron process. Seems like that might be a good place to put perf tests.

@dpwatrous
Copy link
Member

Looks like there's a styling issue with errors on the demo page (this definitely wouldn't pass color contrast requirements):

Screenshot 2023-09-12 at 11 08 59 AM

@wanghoppe
Copy link
Member Author

wanghoppe commented Sep 15, 2023

Looks like there's a styling issue with errors on the demo page (this definitely wouldn't pass color contrast requirements):

Screenshot 2023-09-12 at 11 08 59 AM

I use the latest @fluentui/azure-themes instead, still need to make some adjustment to make the error message looked OK. Also adjust some style to make the shimmering more promient on dark and light theme.
image

packages/bonito-ui/package.json Outdated Show resolved Hide resolved
web/dev-server/resources/i18n/resources.en.json Outdated Show resolved Hide resolved
@wanghoppe wanghoppe enabled auto-merge (squash) September 27, 2023 05:47
@wanghoppe wanghoppe merged commit c947aff into main Oct 17, 2023
6 checks passed
@wanghoppe wanghoppe deleted the hoppe/datagrid2 branch October 17, 2023 00:37
gingi pushed a commit that referenced this pull request Nov 9, 2023
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.

None yet

2 participants