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.

Sign up for GitHub

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

Jump to bottom

feat: Use count for scrolling size, if available #1473

Merged
merged 33 commits into from
Dec 13, 2023
Merged

feat: Use count for scrolling size, if available #1473

merged 33 commits into from
Dec 13, 2023

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Oct 6, 2023 •
edited by krissvaa
Loading

Also fixes #1467
Add support to show the "Total Number Of Records" for the grid optionally

@sonarcloud
Copy link

sonarcloud bot commented Oct 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Oct 6, 2023 •
edited
Loading

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (c3f7fd5) 94.94% compared to head (5a39d75) 95.01%.

Files Patch % Lines
packages/ts/react-crud/src/autogrid.tsx 94.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1473      +/-   ##
==========================================
+ Coverage   94.94%   95.01%   +0.07%     
==========================================
  Files          51       51              
  Lines        3361     3411      +50     
  Branches      486      496      +10     
==========================================
+ Hits         3191     3241      +50     
  Misses        140      140              
  Partials       30       30
Flag Coverage Δ
unittests 95.01% <95.38%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tarekoraby tarekoraby added this to the 2.5 milestone Nov 6, 2023
@krissvaa krissvaa force-pushed the use-count-if-available branch 2 times, most recently from fb2d3fc to 0f683d8 Compare November 16, 2023 06:56
@krissvaa
Copy link
Contributor

There is a problem with the double border on top because of the extra column group wrapper (extra hidden header).

@krissvaa krissvaa marked this pull request as ready for review November 16, 2023 08:52
taefi
taefi requested changes Nov 22, 2023
View reviewed changes
packages/ts/react-crud/src/autogrid.tsx Outdated Show resolved Hide resolved
packages/ts/react-crud/src/autogrid.tsx Outdated Show resolved Hide resolved
packages/ts/react-crud/src/autogrid-renderers.tsx Outdated Show resolved Hide resolved
packages/ts/react-crud/src/autogrid.tsx Outdated Show resolved Hide resolved
taefi
taefi previously requested changes Nov 23, 2023
View reviewed changes
packages/ts/react-crud/test/autogrid.spec.tsx Outdated Show resolved Hide resolved
packages/ts/react-crud/test/autogrid.spec.tsx Outdated Show resolved Hide resolved
packages/ts/react-crud/src/autogrid-renderers.tsx Outdated Show resolved Hide resolved
@krissvaa krissvaa marked this pull request as draft November 28, 2023 08:42
@krissvaa krissvaa force-pushed the use-count-if-available branch 4 times, most recently from 6da01ab to 630ae33 Compare November 29, 2023 11:55
@krissvaa krissvaa marked this pull request as ready for review November 29, 2023 13:13
@krissvaa krissvaa requested a review from taefi November 29, 2023 14:48
@taefi
Copy link
Contributor

taefi commented Nov 30, 2023

Would it be possible to fix the sonar bug and code smells before we merge?

Artur- and others added 10 commits November 30, 2023 17:57
@Artur- @krissvaa
feat: Use count for scrolling size, if available
443b182
@krissvaa
feat(autgrid): Add total and record count to autogrid footer
093a144 
Fixes #1467
@krissvaa
feat(autgrid): Rename record count to filtered count
cd1e99b 
Simplify autogrid renderer function

Fixes #1467
@krissvaa
feat(autgrid): (WIP) Move to more react like implementation of row co…
79a5f1f 
…unt in footer

* Update only footer after updating size
* Simplify setting of grid size

Fixes #1467
@krissvaa
feat(autgrid): Fix tests
887e482 
Fixes #1467
@krissvaa
feat(autgrid): Update grid row count method name
91ae67c
@krissvaa
feat(autgrid): Fix re-render and update custom renderer to use props
d51053c 
*UseEffect is called after data provider was updated, so it was simplified to be called on initial render

Fixes #1467
@krissvaa
feat(autgrid): Change total and filter count message in footer
3c498de 
Fixes #1467
@krissvaa
feat(autgrid): Restore itemIdProperty
86f18d6 
Fixes #1467
@krissvaa
feat(autgrid): Refactor methods to have less complexity
6794cda 
*remove unused imports

Fixes #1467
@taefi
Merge branch 'main' into use-count-if-available
fd2df09
@taefi
Merge branch 'main' into use-count-if-available
ec103c7
@taefi
fix provided tests
2c8b9cb
@taefi
Merge branch 'main' into use-count-if-available
fd1d329
@taefi
only render counts when count is available
0c20772
@taefi
Merge branch 'main' into use-count-if-available
0e774a2
@taefi
fix code smells
a909571
@taefi
add tests for add remove item and refresh effect on counts
8da3b4b
@taefi
Merge branch 'main' into use-count-if-available
d735881
@taefi
add test to verify initializing dataProvider with infinite scrolling …
cc55771 
…works
@taefi
Merge remote-tracking branch 'origin/use-count-if-available' into use…
fbd9d12 
…-count-if-available
@taefi
Merge branch 'main' into use-count-if-available
6f98ab5
@taefi
remove unused const
fda6d60
@taefi
Merge remote-tracking branch 'origin/use-count-if-available' into use…
d9d11e1 
…-count-if-available
sissbruecker
sissbruecker reviewed Dec 12, 2023
View reviewed changes
packages/ts/react-crud/src/autogrid.tsx Outdated Show resolved Hide resolved
packages/ts/react-crud/src/autogrid.tsx Outdated Show resolved Hide resolved
@taefi taefi dismissed their stale review December 12, 2023 14:50

contributer to this PR

@taefi taefi requested review from sissbruecker and removed request for taefi December 12, 2023 14:51
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 13, 2023

Quality Gate Failed Quality Gate failed

Failed conditions

4.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@taefi taefi merged commit 590d5e7 into main Dec 13, 2023
14 of 15 checks passed
@taefi taefi deleted the use-count-if-available branch December 13, 2023 10:35
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Hilla 24.4.0.alpha1 and is also targeting the upcoming stable 24.4.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taefi taefi taefi left review comments

@sissbruecker sissbruecker sissbruecker approved these changes

Assignees
No one assigned
Labels
prerelease version for Hilla 24.4.0 Released with Hilla 24.4.0.alpha1
Projects
None yet
Milestone
2.5
Development

Successfully merging this pull request may close these issues.

[autogrid] support showing the "Total Number Of Records" for the grid optionally
6 participants
@Artur- @krissvaa @taefi @vaadin-bot @sissbruecker @tarekoraby