yeatmanlab / roar-dashboard

A dashboard to administer ROAR assessments
https://roar.education
Other
4 stars 4 forks source link

WIP: Task Grouping on Score Report #717

Closed Emily-ejag closed 2 months ago

Emily-ejag commented 3 months ago

Proposed changes

THIS IS THE INSPIRATION

image

Types of changes

What types of changes does this pull request introduce?

Checklist

Justification of missing checklist items

Further comments

github-actions[bot] commented 3 months ago

Visit the preview URL for this PR (updated for commit 34750fe):

https://roar-staging--pr717-enh-task-grouping-sc-vg7qtp0r.web.app

(expires Mon, 29 Jul 2024 19:09:55 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2631e9c58fd0104ecbfddd72a62245ddac467460

cypress[bot] commented 3 months ago

Passing run #4382 ↗︎

0 26 0 0 Flakiness 0

Details:

Tests for PR 717 "WIP: Task Grouping on Score Report" from commit "34750fe5c3548...
Project: roar-dashboard-e2e Commit: 34750fe5c3
Status: Passed Duration: 05:48 💡
Started: Jul 23, 2024 4:17 PM Ended: Jul 23, 2024 4:23 PM

Review all test suite changes for PR #717 ↗︎

Emily-ejag commented 2 months ago

This looks great. I'm approving this here but have a few concerns that we (@Emily-ejag, @kellyel, and I) should discuss at some point but that don't need to block this PR.

I'm concerned about the amount of score/progress report specific code that is going into the RoarDataTable component. Is it time to create a separate RoarScoreTable component? Or is there a better way to modularize all of the score/progress specific stuff so that it doesn't live in a data table component that is also used for user and org tables?

Hey @richford I think it is good to keep it on RoarDataTable, we pass the things that we need on a prop. So if we need this for progress report in the future, we can use the same thing