Skip to content

Conversation

@imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Jun 6, 2025

Part of #3787

Notes for review:

  • Have to be owner or admin in a project (or site admin) to see stats
  • Replaced <Grid>s and <Lists>s with some combination of <Grid2>s and <Table*>`s.
  • To get data to view in the graphs, add workshop dates to the schedule in project settings and use Data Entry to add words with multiple users
  • With npm start you can simulate collection across multiple days by using MongoDB Compass to manually change the dates within CombineDatabase > FrontierCollection > query { projectId: "id-string-of-the-proj" } > pick a word > senses > [0] > SemanticDomains > [0] > created: "double-click-between-the-quotes-to-edit" > change the date > click Update

Table before'n'after screenshots

1600px wide
Screenshot 2025-06-19 085135
Screenshot 2025-06-19 085152

700px wide
Screenshot 2025-06-19 085327
Screenshot 2025-06-19 085337

450px wide
Screenshot 2025-06-19 085447
Screenshot 2025-06-19 085427


This change is Reviewable

@imnasnainaec imnasnainaec self-assigned this Jun 6, 2025
@imnasnainaec imnasnainaec added enhancement New feature or request frontend labels Jun 6, 2025
@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.19%. Comparing base (1ecd39a) to head (5d1e069).
⚠️ Report is 24 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3855      +/-   ##
==========================================
- Coverage   74.20%   74.19%   -0.02%     
==========================================
  Files         288      288              
  Lines       10623    10617       -6     
  Branches     1323     1323              
==========================================
- Hits         7883     7877       -6     
  Misses       2353     2353              
  Partials      387      387              
Flag Coverage Δ
backend 85.41% <ø> (ø)
frontend 65.68% <100.00%> (-0.04%) ⬇️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@imnasnainaec imnasnainaec changed the title [Statistics] Migrate from deprecated Grid2 and use a Table [Statistics] Migrate from deprecated Grid and use a Table Jun 17, 2025
@hahn-kev
Copy link
Contributor

That new grid looks much better than the old one! Especially on smaller screens.

I don't know the react code enough to comment on it.

Scope creep: the Data Cleanup button at 700px doesn't look great, could it just say "Cleanup"?

@jasonleenaylor
Copy link
Contributor

src/components/Statistics/tests/DomainStatistics.test.tsx line 48 at r2 (raw file):

  test("all rows are present", async () => {
    const listItems = screen.queryAllByRole("row");
    expect(listItems.length).toEqual(mockSemanticDomainCountArray.length + 1);

Maybe add a comment here explaining that the query returns a row for each item and one row in the header? Same in the Statistics tests.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 7 files at r1, 3 of 4 files at r2, all commit messages.
Reviewable status: 6 of 7 files reviewed, all discussions resolved (waiting on @imnasnainaec)

Copy link
Collaborator Author

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Yeah, it doesn't look great when it's the only button to wrap to two lines, but handling that properly would require evaluating and/or limiting the string length in all UI languages, which isn't worth it at this time.

Reviewable status: 5 of 7 files reviewed, all discussions resolved (waiting on @imnasnainaec and @jasonleenaylor)

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

@imnasnainaec imnasnainaec merged commit d7e37be into master Jul 10, 2025
28 of 33 checks passed
@imnasnainaec imnasnainaec deleted the grid2b-stats branch July 10, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants