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.

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

Migrate Records Page to React #10623

Open
wants to merge 76 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
bb687c0
WIP transform rankings page to records page
FinnIckler Jan 14, 2025
a3195c1
port filter changes from other pr
FinnIckler Jan 14, 2025
d781932
fix records API
FinnIckler Jan 14, 2025
bd3ca24
add more show variables
FinnIckler Jan 14, 2025
7625d3f
fix after merge
FinnIckler Jan 14, 2025
7d12e2e
use init function
FinnIckler Jan 14, 2025
743750d
implement slim table
FinnIckler Jan 14, 2025
c4b7cf5
Merge branch 'main' into react/records
FinnIckler Jan 15, 2025
e349987
replace fixed with single line
FinnIckler Jan 15, 2025
85db683
separate view
FinnIckler Jan 15, 2025
d47e8ba
history view
FinnIckler Jan 15, 2025
b3bf9d2
implement mixed History
FinnIckler Jan 15, 2025
a577979
id to query to fix duplicates
FinnIckler Jan 15, 2025
f98cedc
refator Headers and Rows
FinnIckler Jan 15, 2025
ae217b4
create dynamic header
FinnIckler Jan 15, 2025
cbd107c
refactor common cells
FinnIckler Jan 15, 2025
f7ce2ba
Merge branch 'main' into react/records
FinnIckler Jan 15, 2025
525da5a
remove unneeded props
FinnIckler Jan 15, 2025
7cabe81
add back accidental deletion of <CountryCell country={country} />
FinnIckler Jan 15, 2025
9c9fa3c
review changes
FinnIckler Jan 20, 2025
91848c1
Merge branch 'main' into react/records
FinnIckler Jan 20, 2025
a919edc
Create dedicated component per table type
kr-matthews Jan 21, 2025
de9b8ae
Fix default 'show' value
kr-matthews Jan 21, 2025
9ad4b6b
Rename TableWrapper -> RecordsTable
kr-matthews Jan 21, 2025
4f1f346
Add key to SlimRecordsTable
kr-matthews Jan 21, 2025
bfa5c44
Add key to RankingTypeTable
kr-matthews Jan 21, 2025
ab22328
Add key to HistoryRecordsTable
kr-matthews Jan 21, 2025
898b49a
Use filter in table rendering
kr-matthews Jan 21, 2025
50ac182
Create util file
kr-matthews Jan 21, 2025
f345a18
Rename components to reflect mult. tables
kr-matthews Jan 21, 2025
a55c885
Remove unnecessary key
kr-matthews Jan 21, 2025
230480d
Rename/simplify tables
kr-matthews Jan 21, 2025
b39295c
Create and use RecordsTable
kr-matthews Jan 21, 2025
69a8546
Rename to isMixed
kr-matthews Jan 21, 2025
ce63f1b
Formatting
kr-matthews Jan 21, 2025
dc4fa9c
add empty results fallback
FinnIckler Jan 21, 2025
3cf7223
add more linebreaks
FinnIckler Jan 21, 2025
348f6e6
add getRegionIdWithFallback
FinnIckler Jan 21, 2025
2914644
add empty results fallback
FinnIckler Jan 21, 2025
b60f906
add getRegionIdWithFallback
FinnIckler Jan 21, 2025
a11bda1
Merge branch 'kr-matthews-react/records' into react/records
FinnIckler Jan 21, 2025
7ae9c73
lint
FinnIckler Jan 21, 2025
ad8b79e
Merge branch 'main' into react/records
gregorbg Feb 3, 2025
d8b0cc4
Fix import after merging
gregorbg Feb 3, 2025
0e18de3
Combine 'ResultsFilter' after merge
gregorbg Feb 3, 2025
7ebbca1
Styling: Remove line break from 'All' button
gregorbg Feb 3, 2025
9717007
Show filters table while fetching data
gregorbg Feb 3, 2025
e326553
Add cubing icon to sub-table headers
gregorbg Feb 3, 2025
a51dfca
Use shared cell models
gregorbg Feb 3, 2025
92ae2be
Add cache iso2 backwards compatibility
gregorbg Feb 3, 2025
7470bf8
Make sure the iso2 fallback is only used wheen necessary
gregorbg Feb 3, 2025
d32318a
Merge branch 'main' into react/records
gregorbg Feb 4, 2025
6577a27
WIP Integrate Tanstack Table rendering mechanism
gregorbg Feb 4, 2025
61ca071
Collapse Records tables into reusable table skeletons
gregorbg Feb 5, 2025
2f9238f
Fix augmenting flakes
gregorbg Feb 5, 2025
257645a
Fix Slim records view key quirk
gregorbg Feb 5, 2025
335bb2a
Provide React-Table configs for every Records view
gregorbg Feb 5, 2025
735424e
Be economical about augmenting slim data
gregorbg Feb 5, 2025
30c532c
Share column definitions between Rankings & Records
gregorbg Feb 5, 2025
e9aebc9
Hack rendering exception for multi-column cell
gregorbg Feb 5, 2025
2dd74f7
Use accessorKey for slim averages
gregorbg Feb 5, 2025
5372707
Fix duplicate key issue in history view
gregorbg Feb 5, 2025
f2ced0c
Shuffle column def code around
gregorbg Feb 5, 2025
e299904
Fix results DNF/DNS quirk
gregorbg Feb 5, 2025
b18d903
Refactor controller results grouping into common method
gregorbg Feb 5, 2025
618d7bf
Fix null pointer for tied average records in Slim view
gregorbg Feb 5, 2025
ae8dafc
No pop-ups for Country Flags
gregorbg Feb 5, 2025
ea8afcb
Revert unrelated change on CompetitionsFilteers
gregorbg Feb 6, 2025
72bf966
Update app/webpacker/components/Results/ResultsFilter.jsx
gregorbg Feb 6, 2025
aa0d240
Review: Use 'clear' button instead of 'all' button in events selector
gregorbg Feb 6, 2025
4bbe0a4
Review: Remove manually computed 'key'
gregorbg Feb 6, 2025
3dfb4f0
Review: Code cleanup
gregorbg Feb 6, 2025
3da44c5
Consider DNF/DNS as worst attempt
gregorbg Feb 6, 2025
e5f0a6a
Review: move Rankings selector fn to separatte utils file
gregorbg Feb 6, 2025
645ca87
Move over config as well so that the selector fn in utils doesn't fee…
gregorbg Feb 6, 2025
b435069
ESLint
gregorbg Feb 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 55 additions & 16 deletions app/controllers/results_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,22 +161,8 @@ def rankings

@ranking_timestamp = ComputeAuxiliaryData.successful_start_date || Date.current

respond_to do |format|
format.html {}
format.json do
cached_data = Rails.cache.fetch ["results-page-api", *@cache_params, @ranking_timestamp] do
rows = DbHelper.execute_cached_query(@cache_params, @ranking_timestamp, @query)
comp_ids = rows.map { |r| r["competitionId"] }.uniq
if @is_by_region
rows = compute_rankings_by_region(rows, @continent, @country)
end
competitions_by_id = Competition.where(id: comp_ids).index_by(&:id).transform_values { |comp| comp.as_json(methods: %w[], include: [], only: %w[cellName id countryId]) }
{
rows: rows.as_json, competitionsById: competitions_by_id
}
end
render json: cached_data
end
respond_from_cache("results-page-api") do |rows|
@is_by_region ? compute_rankings_by_region(rows, @continent, @country) : rows
end
end

Expand Down Expand Up @@ -217,6 +203,7 @@ def records
DAY(competition.start_date) day,
event.id eventId,
event.name eventName,
result.id id,
result.type type,
result.value value,
result.formatId formatId,
Expand Down Expand Up @@ -261,6 +248,12 @@ def records
`rank`, type DESC, start_date, roundTypeId, personName
SQL
end

@record_timestamp = ComputeAuxiliaryData.successful_start_date || Date.current

respond_from_cache("records-page-api") do |rows|
@is_slim || @is_separate ? compute_slim_or_separate_records(rows) : rows
end
end

private def current_records_query(value, type)
Expand Down Expand Up @@ -306,6 +299,27 @@ def records
SQL
end

private def compute_slim_or_separate_records(rows)
FinnIckler marked this conversation as resolved.
Show resolved Hide resolved
single_rows = []
average_rows = []
rows
.group_by { |row| row["eventId"] }
.each_value do |event_rows|
singles, averages = event_rows.partition { |row| row["type"] == "single" }
balance = singles.size - averages.size
if balance < 0
singles += Array.new(-balance, nil)
elsif balance > 0
averages += Array.new(balance, nil)
end
single_rows += singles
average_rows += averages
end

slim_rows = single_rows.zip(average_rows)
[slim_rows, single_rows.compact, average_rows.compact]
end

private def shared_constants_and_conditions
@years = Competition.non_future_years
@types = ["single", "average"]
Expand Down Expand Up @@ -415,4 +429,29 @@ def records
rows_to_display = world_rows + continents_rows + countries_rows
[rows_to_display, first_continent_index, first_country_index]
end

private def respond_from_cache(key_prefix, &)
respond_to do |format|
format.html {}
format.json do
cached_data = Rails.cache.fetch [key_prefix, *@cache_params, @record_timestamp] do
rows = DbHelper.execute_cached_query(@cache_params, @record_timestamp, @query)

# First, extract unique competitions
comp_ids = rows.map { |r| r["competitionId"] }.uniq
competitions_by_id = Competition.where(id: comp_ids)
.index_by(&:id)
.transform_values { |comp| comp.as_json(methods: %w[country], include: [], only: %w[cellName id]) }

# Now that we've remembered all competitions, we can safely transform the rows
rows = yield rows if block_given?

{
rows: rows.as_json, competitionsById: competitions_by_id
}
end
render json: cached_data
end
end
end
end
21 changes: 0 additions & 21 deletions app/helpers/results_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,6 @@ def historical_pb_markers(results)
end
end

def compute_slim_or_separate_records(rows)
single_rows = []
average_rows = []
rows
.group_by { |row| row["eventId"] }
.each_value do |event_rows|
singles, averages = event_rows.partition { |row| row["type"] == "single" }
balance = singles.size - averages.size
if balance < 0
singles += Array.new(-balance, nil)
elsif balance > 0
averages += Array.new(balance, nil)
end
single_rows += singles
average_rows += averages
end

slim_rows = single_rows.zip(average_rows)
[slim_rows, single_rows.compact, average_rows.compact]
end

def pb_type_class_for_result(regional_record, pb_marker)
if pb_marker
record_class = 'pb'
Expand Down
59 changes: 2 additions & 57 deletions app/views/results/records.html.erb
Original file line number Diff line number Diff line change
@@ -1,64 +1,9 @@
<% provide(:title, t(".title")) %>

<% record_timestamp = ComputeAuxiliaryData.successful_start_date || Date.current %>
<% @rows = DbHelper.execute_cached_query(@cache_params, record_timestamp, @query) %>

<div class="container">
<h1><%= yield(:title) %></h1>
<p><%= t("results.last_updated_html", timestamp: wca_local_time(record_timestamp)) %></p>
<p><%= t("results.last_updated_html", timestamp: wca_local_time(@record_timestamp)) %></p>
<p><i><%= t('results.filters_fixes_underway') %></i></p>

<div id="results-selector" class="results-select form-inline">
<%= render 'results_selector', show_records_options: true %>
</div>

<% cache [*@cache_params, record_timestamp, I18n.locale] do %>
<%
comp_ids = @rows.map { |r| r["competitionId"] }.uniq
unless @is_slim
@competitions_by_id = Hash[Competition.where(id: comp_ids).map { |c| [c.id, c] }]
end
%>

<div id="search-results" class="results">
<div id="results-list">
<% if @is_mixed %>
<% @rows.group_by { |row| row["eventId"] }.each do |event_id, rows| %>
<% event = Event.c_find(event_id) %>
<h2>
<%= link_to rankings_path(event.id, "single"), class: "plain" do %>
<%= cubing_icon event.id %>
<%= event.name %>
<% end %>
</h2>
<%= render 'records_mixed_table', rows: rows %>
<% end %>
<% elsif @is_slim || @is_separate %>
<% slim_rows, single_rows, average_rows = compute_slim_or_separate_records(@rows) %>
<% if @is_slim %>
<%= render 'records_slim_table', rows: slim_rows %>
<% else %>
<h2> <%= t("results.table_elements.type_options.single") %> </h2>
<%= render 'records_separate_table', rows: single_rows, type: "single" %>

<h2> <%= t("results.table_elements.type_options.average") %> </h2>
<%= render 'records_separate_table', rows: average_rows, type: "average" %>
<% end %>
<% elsif @is_history %>
<% @rows.group_by { |row| row["eventId"] }.each do |event_id, rows| %>
<% event = Event.c_find(event_id) %>
<h2>
<%= link_to rankings_path(event.id, "single"), class: "plain" do %>
<%= cubing_icon event.id %>
<%= event.name %>
<% end %>
</h2>
<%= render 'records_histories_table', rows: rows %>
<% end %>
<% elsif @is_mixed_history %>
<%= render 'records_histories_table', rows: @rows %>
<% end %>
</div>
</div>
<% end %>
<%= react_component("Results/Records") %>
</div>
50 changes: 50 additions & 0 deletions app/webpacker/components/Results/DataTable.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { flexRender, getCoreRowModel, useReactTable } from '@tanstack/react-table';
import { Table } from 'semantic-ui-react';
import React from 'react';

export default function DataTable({ rows, config }) {
const table = useReactTable({
data: rows || [],
columns: config,
getCoreRowModel: getCoreRowModel(),
});

return (
<div style={{ overflowX: 'scroll' }}>
<Table basic="very" compact="very" striped unstackable singleLine>
<Table.Header>
{table.getHeaderGroups().map((headerGroup) => (
<Table.Row key={headerGroup.id}>
{headerGroup.headers.map((header) => (
<Table.HeaderCell key={header.id} colSpan={header.column.columnDef.colSpan}>
{header.isPlaceholder
? null
: flexRender(header.column.columnDef.header, header.getContext())}
</Table.HeaderCell>
))}
</Table.Row>
))}
</Table.Header>
<Table.Body>
{table.getRowModel().rows.map((row) => (
<Table.Row key={row.id}>
{row.getVisibleCells().map((cell) => (
cell.column.columnDef.isMultiAttemptsHack
? (
<React.Fragment key={cell.id}>
Comment on lines +32 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.
image

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the way the 'hack' ends up looking is the best, with a single column of span 5. I would leave it as-is then. Possibly renaming isMultiAttemptsHack to rendersOwnCells or something like that, which explains the fragment replacing the cell.

{flexRender(cell.column.columnDef.cell, cell.getContext())}
</React.Fragment>
)
: (
<Table.Cell key={cell.id}>
{flexRender(cell.column.columnDef.cell, cell.getContext())}
</Table.Cell>
)
))}
</Table.Row>
))}
</Table.Body>
</Table>
</div>
);
}
Loading