Migrate Records Page to React #10623

FinnIckler · 2025-01-14T14:54:32Z

Still need to do some of the other table types, but it's very similar to the rankings page

FinnIckler · 2025-01-14T15:52:01Z

Here is the slim Table (events are currently not links because the URL is in the other PR)

FinnIckler · 2025-01-15T10:28:15Z

Separate View

FinnIckler · 2025-01-15T14:20:26Z

Added mixed history and history

This means this is now feature complete, but there is definitely possibilities of refactors as there is a lot of code reuse

FinnIckler · 2025-01-15T15:39:08Z

Did a lot of refactoring to reduce code reuse

kr-matthews · 2025-01-19T18:58:14Z

app/webpacker/components/CompetitionsOverview/CompetitionsFilters.js

-      <Button.Group id="state" size="small" compact primary>
+      <Button.Group id="state" size="large" primary>


Is this intentional? It's unrelated to the records page.

It's a change that Gregor did in the other PR and it affects the styling of the filter box. I will remove it when the other PR is merged

I did? 😇 Please point me in the right direction, or at least explain when you want to undo it

I don't think this file should have any changes at this point?

app/webpacker/components/wca/EventSelector.jsx

app/webpacker/components/Results/Records/index.jsx

app/webpacker/components/Results/Records/SlimRecordsTable.jsx

app/webpacker/components/Results/Records/SeparateRecordsTable.jsx

kr-matthews · 2025-01-19T19:48:56Z

app/webpacker/components/Results/TableHeaders.jsx

+import { Table } from 'semantic-ui-react';
+import I18n from '../../lib/i18n';
+
+const headerConfig = {


Slightly confused why there are 4 configs and 4 header components but 5 'show categories'. Probably mixed-history re-uses either mixed or history, but it's not immediate to trace down.

Yes, history and mixed history have the same headers, just with one more column for events so it has the conditional rendering

Chiming in to note that I'd be happy to see a short comment explaining the Array(4).fill('')s. (Probably one comment to explain all of them is enough)

Can we instead do a single header cell of width 5 (or 3 when appropriate)? FM results don't look nice:

app/webpacker/components/Results/Records/index.jsx

app/controllers/results_controller.rb

gregorbg · 2025-01-20T13:40:35Z

app/webpacker/components/CompetitionsOverview/CompetitionsFilters.js

-      <Button.Group id="state" size="small" compact primary>
+      <Button.Group id="state" size="large" primary>


I did? 😇 Please point me in the right direction, or at least explain when you want to undo it

app/webpacker/components/Results/Records/RecordsTable.jsx

gregorbg · 2025-01-20T13:48:13Z

app/webpacker/components/Results/TableHeaders.jsx

+import { Table } from 'semantic-ui-react';
+import I18n from '../../lib/i18n';
+
+const headerConfig = {


Chiming in to note that I'd be happy to see a short comment explaining the Array(4).fill('')s. (Probably one comment to explain all of them is enough)

app/webpacker/components/Results/api/records.js

app/webpacker/components/wca/EventSelector.jsx

gregorbg · 2025-01-20T13:50:48Z

app/webpacker/lib/requests/routes.js.erb

+  if (region !== 'world') {
+    queryParams.append('region', region);
+  }
+
+  if (gender !== 'All') {
+    queryParams.append('gender', gender);
+  }
+
+  if (eventId){
+    queryParams.append('event_id', eventId)
+  }
+
+  if (show){
+    queryParams.append('show', show)
+  }


See my comment on the Rankings PR about redundant-but-not-redundant parameters ;)

kr-matthews

I can/will make some improvements to my recent Competitors tanstack table PR based on what I've seen here.

kr-matthews · 2025-02-06T05:19:25Z

app/webpacker/components/CompetitionsOverview/CompetitionsFilters.js

-      <Button.Group id="state" size="small" compact primary>
+      <Button.Group id="state" size="large" primary>


I don't think this file should have any changes at this point?

app/webpacker/components/Results/ResultsFilter.jsx

kr-matthews · 2025-02-06T05:36:27Z

app/webpacker/components/Results/ResultsFilter.jsx

+}
+
+export default function ResultsFilter({
+  filterState, filterActions, showCategories, isRecords,


I think the prop names should be declarative rather than assuming information about the parent. So I'd rename isRecords to selectAllEventsIsAllowed or allowSelectingAllEvents. But then the logic is kind of mixed between this prop and the filterActions, which feels a bit off. And that might change based on the outcome of my previous comment.

app/webpacker/components/Results/ResultsFilter.jsx

app/webpacker/components/Results/Records/GroupedRankingTypesTable.jsx

app/webpacker/components/Results/TableColumns.jsx

kr-matthews · 2025-02-06T06:04:43Z

app/webpacker/components/Results/DataTable.jsx

+                cell.column.columnDef.isMultiAttemptsHack
+                  ? (
+                    <React.Fragment key={cell.id}>


I was wondering how this was going to work - and seems the answer is 'not well'.

I also thought this was some kind of exception for mbf for a long time.

Hm... I could play around with trying to align it using a sub-header, but if the answer is not forthcoming after ~30 minutes I will just let it be.

Hm, it is easy to specify but cumbersome to style. If you have a definition similar to

[ { accessorKey: 'competitionName' }, { accessorKey: 'personName' }, { header: 'Successful solves', columns: [ { accessorKey: 'value1' }, { accessorKey: 'value2' }, { accessorKey: 'value3' }, ] } ]

then Tanstack pushes the competition name and person name header down to be on the same level as the header for value1, value2, etc.

In other words, it always pushes down individual column headings as far down as possible. If we could instead set this to pull them up as far as possible, I can adjust the rowSpan to make it look nice.

Couldn't find anything in their documentation, so I will go ahead and raise an Issue / discussion

Seems there is TanStack/table#5051 but no official word on whether or when this will be officially supported.

Okay, I managed to implement something based on the discussion linked above:

I think the header margins make this look a bit bloated... Also, it's gonna be tricky figuring out which results to wrap

app/webpacker/components/Results/Rankings/RankingsTable.jsx

Co-authored-by: Kevin Matthews <[email protected]>

…l lonely

FinnIckler added 5 commits January 14, 2025 15:49

WIP transform rankings page to records page

bb687c0

port filter changes from other pr

a3195c1

fix records API

d781932

add more show variables

bd3ca24

fix after merge

7625d3f

github-actions bot assigned FinnIckler Jan 14, 2025

FinnIckler added 2 commits January 14, 2025 16:17

use init function

7d12e2e

implement slim table

743750d

FinnIckler added 3 commits January 15, 2025 10:42

Merge branch 'main' into react/records

c4b7cf5

replace fixed with single line

e349987

separate view

85db683

FinnIckler added 3 commits January 15, 2025 14:17

history view

d47e8ba

implement mixed History

b3bf9d2

id to query to fix duplicates

a577979

FinnIckler added 4 commits January 15, 2025 16:04

refator Headers and Rows

f98cedc

create dynamic header

ae217b4

refactor common cells

cbd107c

Merge branch 'main' into react/records

f7ce2ba

FinnIckler added 2 commits January 15, 2025 16:41

remove unneeded props

525da5a

add back accidental deletion of <CountryCell country={country} />

7cabe81

kr-matthews requested changes Jan 19, 2025

View reviewed changes

FinnIckler added 2 commits January 20, 2025 13:56

review changes

9c9fa3c

Merge branch 'main' into react/records

91848c1

gregorbg reviewed Jan 20, 2025

View reviewed changes

kr-matthews added 2 commits January 20, 2025 16:21

Create dedicated component per table type

a919edc

Fix default 'show' value

de9b8ae

gregorbg added 14 commits February 3, 2025 22:58

Add cache iso2 backwards compatibility

92ae2be

Make sure the iso2 fallback is only used wheen necessary

7470bf8

Merge branch 'main' into react/records

d32318a

WIP Integrate Tanstack Table rendering mechanism

6577a27

Collapse Records tables into reusable table skeletons

61ca071

Fix augmenting flakes

2f9238f

Fix Slim records view key quirk

257645a

Provide React-Table configs for every Records view

335bb2a

Be economical about augmenting slim data

735424e

Share column definitions between Rankings & Records

30c532c

Hack rendering exception for multi-column cell

e9aebc9

Use accessorKey for slim averages

2dd74f7

Fix duplicate key issue in history view

5372707

Shuffle column def code around

f2ced0c

gregorbg force-pushed the react/records branch from 58e2216 to 2a0eac4 Compare February 5, 2025 09:54

Fix results DNF/DNS quirk

e299904

gregorbg force-pushed the react/records branch from 2a0eac4 to e299904 Compare February 5, 2025 09:57

gregorbg added 3 commits February 5, 2025 19:08

Refactor controller results grouping into common method

b18d903

Fix null pointer for tied average records in Slim view

618d7bf

No pop-ups for Country Flags

ae8dafc

kr-matthews requested changes Feb 6, 2025

View reviewed changes

gregorbg and others added 9 commits February 6, 2025 16:07

Revert unrelated change on CompetitionsFilteers

ea8afcb

Update app/webpacker/components/Results/ResultsFilter.jsx

72bf966

Co-authored-by: Kevin Matthews <[email protected]>

Review: Use 'clear' button instead of 'all' button in events selector

aa0d240

Review: Remove manually computed 'key'

4bbe0a4

Review: Code cleanup

3dfb4f0

Consider DNF/DNS as worst attempt

3da44c5

Review: move Rankings selector fn to separatte utils file

e5f0a6a

Move over config as well so that the selector fn in utils doesn't fee…

645ca87

…l lonely

ESLint

b435069

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Records Page to React #10623

Migrate Records Page to React #10623

FinnIckler commented Jan 14, 2025 •

edited

Loading

FinnIckler commented Jan 14, 2025

FinnIckler commented Jan 15, 2025

FinnIckler commented Jan 15, 2025

FinnIckler commented Jan 15, 2025

kr-matthews Jan 19, 2025

FinnIckler Jan 19, 2025

gregorbg Jan 20, 2025

FinnIckler Jan 20, 2025

kr-matthews Feb 6, 2025

kr-matthews Jan 19, 2025

FinnIckler Jan 19, 2025

gregorbg Jan 20, 2025

kr-matthews Jan 21, 2025

gregorbg Jan 20, 2025

gregorbg Jan 20, 2025

gregorbg Jan 20, 2025

kr-matthews left a comment

kr-matthews Feb 6, 2025

kr-matthews Feb 6, 2025

kr-matthews Feb 6, 2025

gregorbg Feb 6, 2025

gregorbg Feb 6, 2025

gregorbg Feb 6, 2025

gregorbg Feb 6, 2025

gregorbg Feb 6, 2025

gregorbg Feb 6, 2025

		<Button.Group id="state" size="small" compact primary>
		<Button.Group id="state" size="large" primary>

Migrate Records Page to React #10623

Are you sure you want to change the base?

Migrate Records Page to React #10623

Conversation

FinnIckler commented Jan 14, 2025 • edited Loading

FinnIckler commented Jan 14, 2025

FinnIckler commented Jan 15, 2025

FinnIckler commented Jan 15, 2025

FinnIckler commented Jan 15, 2025

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

kr-matthews 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

FinnIckler commented Jan 14, 2025 •

edited

Loading