Grading Overview: TS/Tremor -> AG/Blueprint Migration #2893

InfinityTwo · 2024-03-30T09:01:19Z

Description

This is a major update to the Grading Overview page which migrates from Tanstack Table and Tremor UI to AG Grid and Blueprint JS. It also introduces a few new features that change the user workflow.

Summary

Preferably merged with backend #1107
Frontend does not error without it, but sorting columns will not sort anything.
Part of Migrate all tables to TanStack from AG Grid #2525, Remove Tremor UI library #2849 and AY2023/24 Sem 1 Avenger feedback on Grading Table #2665.
Resolves Grading UI/UX: Grading Table is not Responsive #2804 and Edge case bugs introduced through transfer of pagination of grading submissions from frontend to backend. #2812.
Adds onto Implement isGradingPublished (plus related features) #2856.

Note

For future Avengers and Developers, we would like you to feedback and think about these aspects for future development

XP sorting. Currently sorted by raw XP, but is it preferred over combined XP?
Grading/Filter mode. Is it better to default to Grading or Filter mode? Do you have any default preferences? (Devs can consider session storage for this, and column filters)
Is it better to sort Type by ordering of the tabs at the top (e.g Missions, Quests, Paths, Contests...) or alphabetically?
Is it helpful to add a column for submission date/time?

Please give your feedback in the latest Avenger feedback form/issue tracker or feel free to submit a new issue tracker. Thank you!

Features/Changes Added

Feature/Change 1 - Migration of Table

Part of #2525 and #2849. Adds onto #2856.

All components under Grading Overview will have their components migrated from Tanstack Table and Tremor UI to AG Grid and Blueprint JS. This also means some existing features are modified and may not work the same as before. I have tried my best to keep the table looking the same as before, less the changes made as new features. In #2856, Publish and Unpublish grading buttons were added. I've changed it to become icons to better suit the table. The Unpublish icon can be seen in the picture below, with the first row with the grading icon. The publish icon is at the second row of icons, the last icon.

Feature/Change 2 - Grading/Filter Mode

The default workflow is grading mode. Click on the Grading Mode/Filter Mode button beside the search bar.

In Grading mode, the entire row of each submission (except actions) will be clickable, which navigates the user straight to the submission grading page. This was added as the expected workflow for new users (avengers) was to be able to enter the grading of that submission when clicked, instead of filtering it. The Grading action icon will be removed to declutter the table in this mode.

In Filter Mode, it works the same as the previous table. When you click on any cell that allows filtering, it will filter by the value of that cell throughout the table. The Grading action button will be added back here to avoid the friction of going back to Grading Mode.

Besides the button to differentiate Grading/Filter Mode, when you hover over filterable cells, in Grading Mode, individual cells will not be highlighted/underlined when hovered but in Filter Mode, it will do so.

Filter Mode

Grading Mode

Feature/Change 3 - Customer Column Headers - Sorting & Hideable Columns

Somewhat part of #2665.

On the top of each column header, there are two buttons on the right. The first button allows you to sort in ascending/descending order. The workflow is don't sort by default, click to sort by ascending, click again to sort by descending, and click again to not sort it.

Note that the sorting is done on the backend with this PR #1107

The second button allows you to hide the respective columns. This allows you to view the table with less clutter, and it helps with devices that have a smaller screen.

Custom Headers with filtered columns and cell

Other Minor Changes

(a) Loading Table Overlay

When filtering or searching for new queries, the table will not load in the new data if another query has been made. On the shell, it still queries for the data in the backend and receives every new query like the previous table as this is a purely frontend change.

I have also added a loading animation to show that it's retrieving the requested data. It shows the animation on the first query and hides it when the most recent data from the most recent query has been received.

The table will also become uninteractable (filtering will still be interactable) when querying for the data.

Tl;dr of how it works: when a query is sent, the counter increments by 1. When a query is received, the counter decrements by 1 or stays at 0 if already at 0. If the counter !== 0, then the loading is shown. In certain rare cases, it may not show the loading overlay (still can't figure out why).

(b) Resolves Grading UI/UX: Grading Table is not Responsive #2804 left cutoff, but the fix is not 100% mobile responsive. It will work on mobile, but the user will need to scroll right.
(c) Resolves Edge case bugs introduced through transfer of pagination of grading submissions from frontend to backend. #2812. This introduces a removal of non-"graded" filters when viewing unpublished submissions as that should only show unpublished but graded submissions.
(d) Added a refresh button at the top of the table to refresh the same query (for convenience in case you want to query an updated database with the same options/filters).

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Code quality improvements

How to test

Head over to the Grading page and explore it over there.

Checklist

I have migrated the core components & features
I have cleaned up unrequired files and code
I have modified mock data
I have extensively tested this code
I have successfully merged it with the latest master branch (including P2's isGradingPublished PR) and fixed all conflicts
Backend Sorting has been completed
I have updated the documentation (Not sure if needed)

Switch branch names as changes are more than just filterable columns

… mode, bugfixes

Merge changes cause I forgot to switch branch and this is easier

I need to stop not switch branch

…f tanstack and tremor table

…, removal of old code

…ects

…nto pr/InfinityTwo/2893

RichDom2185

Hi, looking forward for the changes! For now, I have some preliminary comments below:

Thanks for working on this!

RichDom2185 · 2024-04-01T10:04:35Z

src/pages/academy/grading/subcomponents/GradingSubmissionsTable.tsx

In the resolution of merge conflicts, I went to prioritise your changes over the recently merged PRs, thus the current version does not support team assessments (in fact, some of the other files give a compile error).

Can you help take a look? I've attached the fields that were modified by team assessment PR as a code comment.

The basic idea is that now there will be an XOR constraint (either studentName will be populated in case of an individual assessment, or studentNames for team assessment – same for studentUsername and studentUsernames). Thus the table display will fallback to joining the studentNames/studentUsernames field if the other one is empty.

Refer to #2548 for more details.

Please contact me via Telegram if you require any clarifications.

Thanks!

Thanks for merging up with most of the conflicts, I'll let you know if I need help with this one!

I've merged the changes for this in the latest commit, but I'm unsure if the data shown is correct. I have messaged you on this.

RichDom2185 · 2024-04-01T10:06:24Z

src/commons/grading/GradingFlex.tsx

@@ -0,0 +1,56 @@
+declare const twJustifyContentValues: readonly ["justify-start", "justify-end", "justify-center", "justify-between", "justify-around", "justify-evenly"];


Can I check what is this used for?

think I copied it blindly to avoid errors, will look through as well with the other next comment when I get around to looking at it.

This has been fixed in the latest commit, it can be resolved once you are ok with it.

RichDom2185 · 2024-04-01T10:07:42Z

src/commons/grading/GradingFlex.tsx

+    display: "flex",
+    justifyContent: 
+      (justifyContent === "justify-start"
+        ? "flex-start"
+        : justifyContent === "justify-end"
+        ? "flex-end"
+        : justifyContent === "justify-center"
+        ? "center"
+        : justifyContent === "justify-between"
+        ? "space-between"
+        : justifyContent === "justify-around"
+        ? "space-around"
+        : justifyContent === "justify-evenly"
+        ? "space-evenly"
+        : ""
+      ),


Seems like we are mapping Tailwind values to actual styles – what's the need for this? Can we just map input props directly?

I was just being lazy and trying to avoid touching existing props but I can go through all of it again when I work on it to remove it.

This has been fixed in the latest commit, it can be resolved once you are ok with it.

RichDom2185 · 2024-04-01T10:07:54Z

src/commons/grading/GradingFlex.tsx

+  );
+}
+
+export default GradingFlex;


Please add an empty line at the end of this file.

This has been fixed in the latest commit, it can be resolved once you are ok with it.

RichDom2185 · 2024-04-01T10:08:03Z

src/commons/grading/GradingText.tsx

+  );
+}
+
+export default GradingText;


Ditto on newline at EOF

This has been fixed in the latest commit, it can be resolved once you are ok with it.

RichDom2185 · 2024-04-01T10:08:19Z

src/commons/sagas/BackendSaga.ts

@@ -466,6 +484,7 @@ function* BackendSaga(): SagaIterator {
        gradedFilter,
        pageParams,
        filterParams
+        // sortedBy


Is this going to be used/removed?

It will be used for backend column sorting onced implemented, commented for now in case it causes errors.

This has been uncommented in the latest commit and the various issues that comes with it has been fixed.

josh1248 · 2024-04-03T02:55:36Z

does this change have the edge case bugs in #2812? seems likely that aggrid's way of doing things over tanstack could help to curb the forceful frontend filtering and solve the issue.

InfinityTwo · 2024-04-03T05:34:12Z

does this change have the edge case bugs in #2812? seems likely that aggrid's way of doing things over tanstack could help to curb the forceful frontend filtering and solve the issue.

It does indeed have that case as the way data is handled is not under aggrid, but I will provide a (rough frontend) fix it in the next commit.

Edit: The changes have been made, and the second issue in the PR will currently remove the filters for Attempting/Attempted (basically those that aren't Submitted) when selecting ungraded in what to view.

… and PR

…s on selecting ungraded, some refactoring

…ation

coveralls · 2024-04-06T09:19:57Z

Pull Request Test Coverage Report for Build 9361057403

Details

30 of 212 (14.15%) changed or added relevant lines in 17 files are covered.
9 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.2%) to 31.192%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/commons/grading/GradingText.tsx	2	3	66.67%
src/pages/academy/grading/subcomponents/GradingFilterable.tsx	1	2	50.0%
src/pages/academy/teamFormation/subcomponents/TeamFormationBadges.tsx	0	1	0.0%
src/commons/grading/GradingFlex.tsx	2	4	50.0%
src/commons/workspace/WorkspaceActions.ts	5	7	71.43%
src/pages/academy/grading/subcomponents/GradingColumnFilters.tsx	1	3	33.33%
src/commons/workspace/WorkspaceReducer.ts	0	4	0.0%
src/pages/academy/grading/subcomponents/GradingActions.tsx	1	5	20.0%
src/commons/sagas/RequestsSaga.ts	0	5	0.0%
src/commons/sagas/BackendSaga.ts	0	8	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/pages/academy/grading/subcomponents/GradingActions.tsx	1	2.33%
src/pages/academy/grading/Grading.tsx	4	0.0%
src/pages/academy/grading/subcomponents/GradingSubmissionsTable.tsx	4	2.6%

Totals
Change from base Build 9101202777:	-0.2%
Covered Lines:	4924
Relevant Lines:	14811

💛 - Coveralls

…nto pr/InfinityTwo/2893

* Add React import * Remove unnecessary type annotations * Extract default styles outside component * Remove unused props

* Add React import * Use `classnames` utility * Use `Classes` object instead of raw string CSS API * Move constant default styles out of component * Remove unused props

* Use strict inequality * Use `Object.entries` to iterate over both key and value

* Add React import * Use `useSession` over `useTypedSelector` * Use Blueprint's `Tooltip` component over `htmlTitle` * Remove unnecessary `={true}` in props * Simplify conditionals with `&&`

For better readability.

For readability.

* Add React import * Use `classNames` utility instead of `String` * Remove unused prop export * Convert Props interface to type * Remove unnecessary type annotations and arguments * Convert conditional to `&&`

* Add React import

* Add React import * Improve typing of badge colors * Use optional chaining where possible * Remove unused export

Also renamed a type to `Props` as it is unexported.

…nto pr/InfinityTwo/2893

lhw-1

First of all, I appreciate the extensive documentation on the PR itself. Great job! 😄

In terms of the migration & features, I like that the UI was kept consistent despite the migration - thanks for the effort! One additional suggestion here might be to include a tooltip for how Filter Mode vs Grading Mode works, but it's not urgent & can be a separate issue.

Overall, the changes seem reasonable & I don't think I have too many issues with the implementation itself. I have a few small queries / nits on the changes below:

src/commons/grading/GradingFlex.tsx

src/commons/grading/GradingText.tsx

lhw-1 · 2024-05-15T14:49:49Z

src/commons/mocks/BackendMocks.ts

+      };
+
+      Object.keys(allColsSortStates.currentState).forEach(key => {
+        if (allColsSortStates.sortBy === key && key != '') {


Suggested change

if (allColsSortStates.sortBy === key && key != '') {

if (allColsSortStates.sortBy === key && key !== '') {

By convention & keeping it consistent with BackendSaga.ts

Adding on - would there be any issues if we just do key instead of key !== ''?

I've changed it to key since it works too 😄

lhw-1 · 2024-05-15T14:57:38Z

src/features/grading/GradingTypes.ts

+export enum SortStates {
+  ASC = 'sort-asc',
+  DESC = 'sort-desc',
+  NONE = 'sort'


The enum seems to be used now instead of the raw string values 👍

lhw-1 · 2024-05-15T14:59:52Z

src/features/grading/GradingTypes.ts

@@ -53,6 +74,20 @@ export type GradingOverviewWithNotifications = {
 */
 export type GradingAnswer = GradingQuestion[];

+export type AllColsSortStates = {
+  currentState: SortStateProperties;
+  sortBy: ColumnFieldsKeys | '';


Is the | '' necessary?

The empty string allows us to not sort by any columns, which is the default state from my memory

lhw-1 · 2024-05-15T15:01:26Z

src/features/grading/GradingTypes.ts

+  studentUsername = 'Username(s)',
+  groupName = 'Group',
+  progressStatus = 'Progress',
+  xp = 'Raw XP (+Bonus)',


Suggested change

xp = 'Raw XP (+Bonus)',

xp = 'Raw XP (+ Bonus)',

Just a very tiny unimportant personal preference, feel free to reject based on whichever looks better in the UI 😅

I think the one without space looks better in the UI after trying it out, having a space feels a little off

…-frontend into grading-overview-ag-bp-migration

…/InfinityTwo/sourceacademy-frontend into grading-overview-ag-bp-migration

InfinityTwo added 14 commits February 28, 2024 18:09

added filterable columns in grading overview

97a1ae4

some cleanup code

567a471

merge conflicts

0c5df89

moved some grading overview FE components from tremor to blueprint

5a5eaab

missing code from previous commit

852da3c

halfway done for porting tanstack/tremor to ag grid/blueprint

c4f0ddf

Merge pull request #4 from InfinityTwo/grading-filterable-columns

34f8d9a

Switch branch names as changes are more than just filterable columns

more changes to grading table - animation whilst loading, filter/edit…

b22cb6f

… mode, bugfixes

Merge pull request #5 from InfinityTwo/grading-filterable-columns

30014c8

Merge changes cause I forgot to switch branch and this is easier

code refactoring

63932c6

Merge pull request #6 from InfinityTwo/grading-filterable-columns

5a4eb0d

I need to stop not switch branch

filterable columns, backend sorting shell, and some partial removal o…

de27b6d

…f tanstack and tremor table

more component migrations, refactoring and preparation for backend sort

48684a5

multi -> single sorting, moved over all ts/tremor components to ag/bp…

1e7eafc

…, removal of old code

InfinityTwo self-assigned this Mar 30, 2024

InfinityTwo and others added 3 commits March 30, 2024 18:12

fix table cutoff on small horizontal resolution and missing hover eff…

f7021b0

…ects

refresh button for table

e54fe6e

Merge branch 'master' of https://github.com/source-academy/frontend i…

6ae6df5

…nto pr/InfinityTwo/2893

RichDom2185 reviewed Apr 1, 2024

View reviewed changes

InfinityTwo added 8 commits April 3, 2024 13:41

fixed richard's comments (mostly), and added a fix for josh's comment…

c16e3ae

… and PR

fixed edge case of attempted/submitted on filter from prev commit

d1d4a09

mock files, change josh's pr 2nd issue to remove non-submitted filter…

2db1749

…s on selecting ungraded, some refactoring

fixed wrong username and text overflow

83f00cf

eslint

f1f8652

prettier checks

2ed0eec

Merge branch 'source-academy:master' into grading-overview-ag-bp-migr…

2d5cae3

…ation

backend mock changes

886c976

RichDom2185 added 19 commits May 2, 2024 00:51

Merge branch 'master' of https://github.com/source-academy/frontend i…

db2961c

…nto pr/InfinityTwo/2893

Merge branch 'master' into grading-overview-ag-bp-migration

e93a8d5

Merge branch 'master' into grading-overview-ag-bp-migration

04cc13e

Merge branch 'master' of https://github.com/source-academy/frontend i…

8b02ad1

…nto pr/InfinityTwo/2893

Merge branch 'master' of https://github.com/source-academy/frontend i…

86a431b

…nto pr/InfinityTwo/2893

Fix format

c4134c0

Merge branch 'master' of https://github.com/source-academy/frontend i…

f7ea428

…nto pr/InfinityTwo/2893

Fix compile error post-merge

7449edf

Refactor GradingFlex

5428c57

* Add React import * Remove unnecessary type annotations * Extract default styles outside component * Remove unused props

Refactor GradingText

ceebfb9

* Add React import * Use `classnames` utility * Use `Classes` object instead of raw string CSS API * Move constant default styles out of component * Remove unused props

Update BackendSaga.ts

916680c

* Use strict inequality * Use `Object.entries` to iterate over both key and value

Refactor GradingFilterable.tsx

8a1cad8

Refactor GradingActions

847a6ab

* Add React import * Use `useSession` over `useTypedSelector` * Use Blueprint's `Tooltip` component over `htmlTitle` * Remove unnecessary `={true}` in props * Simplify conditionals with `&&`

Simplify conditions in conditionals

1f4108f

For better readability.

Refactor conditions

d2756bc

For readability.

Refactor GradingColumnCustomHeaders.tsx

71eb6d6

* Add React import * Use `classNames` utility instead of `String` * Remove unused prop export * Convert Props interface to type * Remove unnecessary type annotations and arguments * Convert conditional to `&&`

Refactor GradingColumnFilters.tsx

a14c365

* Add React import

Refactor GradingBadges.tsx

70fbc0f

* Add React import * Improve typing of badge colors * Use optional chaining where possible * Remove unused export

Add missing React import

3d1ec43

Also renamed a type to `Props` as it is unexported.

RichDom2185 requested a review from chownces May 12, 2024 08:38

RichDom2185 added 5 commits May 12, 2024 17:58

Merge branch 'master' into grading-overview-ag-bp-migration

0d91ae8

Merge branch 'master' into grading-overview-ag-bp-migration

f79be42

Merge branch 'master' into grading-overview-ag-bp-migration

da2e269

Merge branch 'master' of https://github.com/source-academy/frontend i…

fa7a309

…nto pr/InfinityTwo/2893

Fix imports and format post merge

14b8360

lhw-1 reviewed May 15, 2024

View reviewed changes

lhw-1 and others added 4 commits May 16, 2024 22:19

Merge branch 'master' into grading-overview-ag-bp-migration

e8e68c5

Merge branch 'master' of https://github.com/InfinityTwo/sourceacademy…

72f301f

…-frontend into grading-overview-ag-bp-migration

hw review changes and child key error fix

e08d9b5

Merge branch 'grading-overview-ag-bp-migration' of https://github.com…

086460d

…/InfinityTwo/sourceacademy-frontend into grading-overview-ag-bp-migration

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grading Overview: TS/Tremor -> AG/Blueprint Migration #2893

Grading Overview: TS/Tremor -> AG/Blueprint Migration #2893

InfinityTwo commented Mar 30, 2024 •

edited

RichDom2185 left a comment

RichDom2185 Apr 1, 2024

InfinityTwo Apr 1, 2024

InfinityTwo Apr 3, 2024

RichDom2185 Apr 1, 2024

InfinityTwo Apr 1, 2024

InfinityTwo Apr 3, 2024

RichDom2185 Apr 1, 2024

InfinityTwo Apr 1, 2024

InfinityTwo Apr 3, 2024

RichDom2185 Apr 1, 2024

InfinityTwo Apr 3, 2024

RichDom2185 Apr 1, 2024

InfinityTwo Apr 3, 2024

RichDom2185 Apr 1, 2024

InfinityTwo Apr 1, 2024

InfinityTwo Apr 3, 2024

josh1248 commented Apr 3, 2024

InfinityTwo commented Apr 3, 2024 •

edited

coveralls commented Apr 6, 2024 •

edited

lhw-1 left a comment •

edited

lhw-1 May 15, 2024

lhw-1 May 15, 2024

InfinityTwo Jun 4, 2024

lhw-1 May 15, 2024

lhw-1 May 15, 2024

InfinityTwo May 15, 2024

lhw-1 May 15, 2024

InfinityTwo Jun 4, 2024

		@@ -0,0 +1,56 @@
		declare const twJustifyContentValues: readonly ["justify-start", "justify-end", "justify-center", "justify-between", "justify-around", "justify-evenly"];

	if (allColsSortStates.sortBy === key && key != '') {
	if (allColsSortStates.sortBy === key && key !== '') {

Grading Overview: TS/Tremor -> AG/Blueprint Migration #2893

Are you sure you want to change the base?

Grading Overview: TS/Tremor -> AG/Blueprint Migration #2893

Conversation

InfinityTwo commented Mar 30, 2024 • edited

Description

Summary

Features/Changes Added

Feature/Change 1 - Migration of Table

Feature/Change 2 - Grading/Filter Mode

Feature/Change 3 - Customer Column Headers - Sorting & Hideable Columns

Other Minor Changes

Type of change

How to test

Checklist

RichDom2185 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh1248 commented Apr 3, 2024

InfinityTwo commented Apr 3, 2024 • edited

coveralls commented Apr 6, 2024 • edited

Pull Request Test Coverage Report for Build 9361057403

Details

💛 - Coveralls

lhw-1 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InfinityTwo commented Mar 30, 2024 •

edited

InfinityTwo commented Apr 3, 2024 •

edited

coveralls commented Apr 6, 2024 •

edited

lhw-1 left a comment •

edited