-
-
Notifications
You must be signed in to change notification settings - Fork 10
🎨 Show tasks by contest types (#1644) #1800
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
Conversation
WalkthroughThe changes refactor task result filtering and table generation for contest types. The component now uses a configuration object to determine the appropriate filtering and table generator based on the active contest type. The previous inline filtering logic and helper functions have been removed in favor of two new utility files. One utility implements an abstract filtering class with concrete subclasses for different contest ranges, while the other provides an abstract table generator class with corresponding implementations. New types for task result filtering and table generation have also been exported. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as TaskTable.svelte
participant F as TaskResultsFilter (Factory)
participant G as TaskTableGenerator (Factory)
U->>T: Select contest type
T->>F: Retrieve filter config for contest type
F->>F: Execute filter logic (run)
F-->>T: Return filtered task results
T->>G: Retrieve table generator for contest type
G->>G: Generate task table (run)
G-->>T: Return table structure
T->>U: Render updated task table
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/components/TaskTables/TaskTable.svelte (1)
86-93: Optional inlining
If desired, calls totaskTableGenerator.getTitle()andtaskTableGenerator.getContestRoundLabel()could be inlined where used, but this helper approach is also fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/TaskTables/TaskTable.svelte(4 hunks)src/lib/utils/task_results_filter.ts(1 hunks)src/lib/utils/task_table_generator.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview
🔇 Additional comments (26)
src/lib/components/TaskTables/TaskTable.svelte (13)
20-25: Imports look good
No issues found with these import statements.
27-33: Imports look consistent
All references to the task table generator utilities are well-structured.
43-66: Configuration object is well-designed
Defining each contest range with both a filter and a table generator ensures clear separation of concerns.
68-69: Union type definition is appropriate
This union type cleanly enumerates valid contest filter keys.
70-71: Reactive state usage
Storing the active filter key is straightforward and easy to maintain.
72-76: Derived filter creation
The reactivity setup for task results filter is clear and follows good Svelte patterns.
78-85: Derived table generator
Consistently mirrors the filter approach. No concerns identified.
127-135: Contest filter buttons
Button rendering logic is tidy, with aria-labels for accessibility.
139-139: Heading usage
Displays the current table title effectively.
149-151: Header cell for "Round"
No issues found.
153-157: Task table headers
Conditional rendering is properly handled for columns.
161-165: Contest row rendering
No issues identified.
168-179: Cell content logic
Conditional checks and modal opening logic are well structured.src/lib/utils/task_results_filter.ts (6)
1-5: Imports and classification function
The dependency setup is clear and aligns with the usage in the filtering logic.
6-19: Factory functions for filter classes
Exposing factory functions simplifies the consumer code and keeps creation logic contained.
20-46: Abstract TaskResultsFilter
A well-defined abstract class for consistent filtering. The documentation comments are thorough.
48-70: TaskResultsForABCLatest20
Correctly narrows the filtered list to the latest 20 contests. Sorting and slicing logic looks spot-on.
72-79: TaskResultsFromABC319Onwards
Simple conditional filtering from abc319 through abc999 is clear.
81-88: TaskResultsFromABC212ToABC318
Logical bounds for filtering ABC212 to ABC318. Implementation is straightforward.src/lib/utils/task_table_generator.ts (7)
1-5: Imports for contest and task utilities
No issues found. References are consistent with usage.
7-26: Generator factory functions
Exposing distinct generator constructors is a clean design and keeps the code flexible.
28-113: Abstract TaskTableGenerator
Good structure for table creation, with O(N) complexity. Sorting and reversing contest IDs is well-implemented.
115-124: Base class for ABC tasks
Straightforward override for contest labeling ingetContestRoundLabel().
126-130: TaskTableGeneratorForABCLatest20
Provides a clear, distinct title for the latest 20 ABC rounds.
132-136: TaskTableGeneratorFromABC319Onwards
Correctly customizes the title for contests 319 onward.
138-142: TaskTableGeneratorFromABC212ToABC318
Title conveys the relevant contest ID range effectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (10)
src/lib/components/TaskTables/TaskTable.svelte (10)
20-26: Use consistent naming to avoid confusion.
Currently, the imported filter methods (taskResultsForABCLatest20, etc.) suggest both filtering and result retrieval. Consider renaming these functions or clearly documenting that they return filtered task results.
27-33: Consider grouping related imports.
These generator imports are structurally similar to the filter imports. Grouping them under a single grouped import statement (and/or in a separateindex.tsre-export) might reduce repeated references and keep imports concise.
43-44: Address the TODO comments.
The code references two unimplemented TODOs about adding arbitrary contest types and deciding the contest type ordering. Implement them (or track them in the backlog) to maintain clarity and avoid leaving dead comments in production code.Do you want me to propose an approach for dynamically registering new contest types or reordering them?
72-77: Reactive store approach may benefit from inlined usage.
You're creating bothtaskResultsFilterandselectedTaskResultsas derived references. Alternatively, you could inline such logic in a single derived call to reduce store declarations. If you need additional methods from the filter object, though, the current approach is justified.
86-88: Provide a default title.
getTaskTableTitle(taskTableGenerator)returns''if no title is defined. Consider providing a meaningful default (e.g.,"Task Table"), so users see an informative heading rather than a blank.
90-92: Ensure localized or consistent round labeling.
getContestRoundLabelis straightforward but ensure the label respects localization or naming conventions if needed.
128-134: Improve button accessibility & visual feedback.
You're settingariaLabeland toggling an active class. Consider adding focus styles or usingaria-pressedto signal the active filter state, improving accessibility for keyboard and screen reader users.
139-139: Conditional heading styling.
You're displaying the heading with the text "2xl" and "pb-3." Ensure that large screens and smaller devices properly adapt. You might want to use responsive classes (e.g.,text-xl md:text-2xl).
149-151: Consider i18n for column headers.
“Round” is currently hardcoded. If you intend to support multiple languages, consider an i18n approach (e.g., a string from a translation file).
153-155: Avoid potential rendering overhead for large tables.
IftaskTableHeaderIdsgrows large, the repeated{#each}for multiple contest rows might cause performance issues. Virtualization can help if you expect thousands of entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/components/TaskTables/TaskTable.svelte(4 hunks)
🔇 Additional comments (4)
src/lib/components/TaskTables/TaskTable.svelte (4)
68-69: Typings look consistent.
Defining a narrow union type forContestTypeFilterhelps ensure no invalid contest type is selected at runtime.
78-85: Maintain consistent patterns for table generation.
Similar totaskResultsFilter, you createtaskTableGeneratorand derive the final data. This is fine if multiple table generation steps are needed; otherwise, inlining might simplify.
70-71:Details
✅ Verification successful
Confirm the store usage.
$state<ContestTypeFilter>('abcLatest20Rounds')appears to be a custom store approach, which is fine. Ensure that it properly integrates with your state management solution and triggers reactivity as expected.
🏁 Script executed:
#!/bin/bash # Search for "$state<" usage across the codebase to confirm usage consistency rg '\$state<'Length of output: 363
Store usage verified and confirmed.
I ran the search for$state<and found identical usage in several files (e.g., in theTaskTable.svelte,replenishment_workbook.svelte.ts, andUpdatingModal.sveltefiles). This confirms that the custom store syntax is applied consistently across the codebase, and it properly integrates with the state management solution. No further modifications are required.
45-66:Details
❓ Verification inconclusive
Validate custom logic for each contest type.
Each config entry calls a different filter and table generator for ABC rounds. Ensure that any new or future contest types can be easily added and that errors are properly handled if the returned dataset is empty or missing.Please verify in the codebase whether all contest types have the associated filters and table generators. You can run:
🏁 Script executed:
#!/bin/bash rg -A 3 'export function taskResultsFor' rg -A 3 'export function taskResultsFrom' rg -A 3 'export function taskTableGeneratorFor' rg -A 3 'export function taskTableGeneratorFrom'Length of output: 187
Confirm the Existence and Robustness of Contest Type Functions
- In the configuration for ABC contest types (e.g.,
abcLatest20Rounds,abc319Onwards, andabc212To318), each entry calls a dedicated filter and table generator (liketaskResultsForABCLatest20,taskTableGeneratorForABCLatest20, etc.).- Automated searches using patterns such as
export function taskResultsForand similar did not return any results. This may mean that these functions are either defined differently (or non-exported) or that similar patterns for other contest types are missing.- Please manually verify that:
- Every contest type (current and future) has an associated filter function and table generator function.
- Each function correctly handles cases where the returned dataset is empty or missing to avoid errors.
close #1644
Summary by CodeRabbit