Make psychsheets sort by average by default #10368
Conversation
| useEffect(() => { | ||
| if (psychSheetEvent === '333mbf') { | ||
| setPsychSheetSortBy('single'); | ||
| } else { | ||
| setPsychSheetSortBy('average'); | ||
| } | ||
| }, [psychSheetEvent]); |
There was a problem hiding this comment.
This is not a good usage of useEffect. In essence, you're only recomputing the initial value for useMemo above. So, you should replace this whole effect hook with a single line along the lines of const default sort = isMultiBld ? 'single' : 'average' and then pass that new value as initializer: useMemo(psychSheetEvent)
| @@ -95,7 +96,7 @@ export default function RegistrationList({ competitionInfo }) { | |||
| const changeSortColumn = (name) => dispatch({ type: 'CHANGE_SORT', sortColumn: name }); | |||
|
|
|||
| const [psychSheetEvent, setPsychSheetEvent] = useState(); | |||
| const [psychSheetSortBy, setPsychSheetSortBy] = useState('single'); | |||
| const psychSheetSortBy = useMemo(() => (psychSheetEvent === '333mbf' ? 'single' : 'average'), [psychSheetEvent]); | |||
There was a problem hiding this comment.
IIRC in the old system all blindfolded events were sorted by single, not just 333mbf
There was a problem hiding this comment.
Thanks for reminding @maxidragon. It's implemented
| @@ -115,7 +117,6 @@ export default function RegistrationList({ competitionInfo }) { | |||
|
|
|||
| const registrationsWithPsychsheet = useMemo(() => { | |||
| if (psychSheet !== undefined) { | |||
| setPsychSheetSortBy(psychSheet.sort_by); | |||
There was a problem hiding this comment.
Why is this line being deleted?
There was a problem hiding this comment.
Otherwise, react throws an error that setPsychSheetSortBy is not defined
There was a problem hiding this comment.
Ah, because you're changing to a useMemo above. I see. But that means when loading a psych sheet from the backend, you cannot change the sort order anymore. I feel like this will break lots of existing use-cases
(for example, this will break proper sorting on events like 333 which have genuine use-cases of using either single or average for sorting)
| @@ -94,8 +94,10 @@ export default function RegistrationList({ competitionInfo }) { | |||
| const { sortColumn, sortDirection } = state; | |||
| const changeSortColumn = (name) => dispatch({ type: 'CHANGE_SORT', sortColumn: name }); | |||
|
|
|||
| const blindFoldedEvents = ['333mbf', '333bf', '444bf', '555bf']; | |||
There was a problem hiding this comment.
You don't need to hardcode this. We have events data available as import { events } from 'lib/wca-data.js.erb' (note you might need to change the relative path to the file)
| const [psychSheetEvent, setPsychSheetEvent] = useState(); | ||
| const [psychSheetSortBy, setPsychSheetSortBy] = useState('single'); | ||
| const psychSheetSortBy = useMemo(() => (blindFoldedEvents.includes(psychSheetEvent) ? 'single' : 'average'), [psychSheetEvent]); |
There was a problem hiding this comment.
Using my suggestion from above, you can compute this as events.byId[psychSheetEvent].recommendedFormat().sortBy
Making new psych sheet behavior consistent with old one