-
Notifications
You must be signed in to change notification settings - Fork 4
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
Remove group option conditionally #138
Conversation
WalkthroughThe changes in this pull request involve several modifications across various components related to tournament management. Key updates include making the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 4
🧹 Outside diff range and nitpick comments (10)
style/lib/pickers/date_and_time_picker.dart (1)
9-9
: LGTM! Consider adding parameter documentation.The addition of the nullable
startDate
parameter is well-placed and aligns with the date validation requirements.Add documentation to clarify the parameter's purpose:
Future<void> selectDate( BuildContext context, { String? helpText, + /// The minimum selectable date. If null, defaults to either initialDate or current date DateTime? startDate, required DateTime initialDate, required Function(DateTime) onDateSelected, })
khelo/lib/ui/flow/tournament/detail/components/flexible_space.dart (1)
47-57
: Add error handling and ensure consistent state managementWhile the implementation is generally good, consider these improvements:
- Add error handling for palette generation
- Ensure consistent mounted checks for all setState calls
Consider this improved implementation:
void _initializeImageProvider(String? imageUrl) { if (imageUrl != null) { imageProvider = CachedNetworkImageProvider(imageUrl); - imageProvider!.createPaletteGenerator().then((generatedPalette) { - if (mounted) { - setState(() => palette = generatedPalette); - } + imageProvider!.createPaletteGenerator().then((generatedPalette) { + if (mounted) { + setState(() => palette = generatedPalette); + } + }).catchError((error) { + // Handle error gracefully, maybe set a default palette + if (mounted) { + setState(() => palette = null); + } }); } else { - setState(() => imageProvider = null); + if (mounted) { + setState(() { + imageProvider = null; + palette = null; + }); + } } }khelo/lib/ui/flow/home/components/tournament_item.dart (1)
117-122
: Simplify null checks and improve safetyThe current implementation has redundant null checks and a forced unwrap that could be simplified.
Consider this more concise and safer approach:
- image: bannerUrl == null || imageProvider == null + image: imageProvider == null ? null : DecorationImage( - image: imageProvider!, + image: imageProvider, fit: BoxFit.fill, ),The
imageProvider
null check alone is sufficient since it can only be non-null whenbannerUrl
is also non-null, based on the initialization logic.khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart (2)
156-164
: Simplify quarterfinal removal logicThe current implementation has redundant checks and could be simplified:
- The outer condition partially overlaps with inner conditions
- Tournament type checks are duplicated
Consider this simplified logic:
- if (tournamentType == TournamentType.doubleOut || - state.matches.containsKey(MatchGroup.quarterfinal)) { - if (tournamentType == TournamentType.doubleOut || - tournamentType == TournamentType.miniRobin || - tournamentType == TournamentType.boxLeague || - tournamentType == TournamentType.knockOut) { - options.remove(MatchGroup.quarterfinal); - } - } + final shouldRemoveQuarterfinal = tournament.type == TournamentType.doubleOut || + tournament.type == TournamentType.miniRobin || + tournament.type == TournamentType.boxLeague || + tournament.type == TournamentType.knockOut || + state.matches.containsKey(MatchGroup.quarterfinal); + if (shouldRemoveQuarterfinal) { + options.remove(MatchGroup.quarterfinal); + }
145-183
: Document business rules and improve structureWhile the implementation correctly handles the match group restrictions, consider these improvements:
- Add documentation explaining the business rules for each tournament type
- Consider using a more maintainable structure for validation rules
Consider this approach:
+ /// Rules for match group availability by tournament type: + /// - Double Out: Maximum 2 rounds, no quarterfinals/semifinals + /// - Mini Robin: No rounds after first, no quarterfinals/semifinals + /// - Box League: No quarterfinals/semifinals + /// - Knock Out: No quarterfinals/semifinals List<MatchGroup> getMatchGroupOption() { + final validationRules = { + TournamentType.doubleOut: { + MatchGroup.round: (matches) => + (matches[MatchGroup.round]?.entries.length ?? 0) >= 2, + MatchGroup.quarterfinal: (_) => true, + MatchGroup.semifinal: (_) => true, + }, + // Add rules for other tournament types + }; // Implementation using validation rules }khelo/lib/ui/flow/tournament/add/add_tournament_view_model.dart (3)
115-120
: LGTM! Consider extracting the duration constant.The date validation logic is correct and aligns with the requirements. However, consider extracting the one-day duration as a named constant for better maintainability.
+ static const Duration MIN_TOURNAMENT_DURATION = Duration(days: 1); void onStartDate(DateTime startDate) { DateTime endDate = state.endDate; if (startDate.isAfter(endDate)) { - endDate = startDate.add(Duration(days: 1)); + endDate = startDate.add(MIN_TOURNAMENT_DURATION); } state = state.copyWith(startDate: startDate, endDate: endDate); onChange(); }
125-134
: Improve readability of date validation logic.While the logic is correct, the nested conditions could be refactored for better readability. Also, the Duration instantiation is repeated.
+ static const Duration MIN_TOURNAMENT_DURATION = Duration(days: 1); void onEndDate(DateTime endDate) { DateTime startDate = state.startDate; if (endDate.isBefore(startDate)) { - startDate = endDate.subtract(Duration(days: 1)); - if (_editedTournament != null && - startDate.isBefore(_editedTournament!.start_date)) { - startDate = _editedTournament!.start_date; - } + startDate = endDate.subtract(MIN_TOURNAMENT_DURATION); + + // For edited tournaments, ensure start date isn't before original + if (_editedTournament?.start_date != null && + startDate.isBefore(_editedTournament!.start_date)) { + startDate = _editedTournament!.start_date; + } } state = state.copyWith(endDate: endDate, startDate: startDate); onChange(); }
115-134
: Consider consolidating date validation logic.Currently, date validation occurs in two places:
- During date selection (
onStartDate
/onEndDate
)- During tournament creation (
addTournament
)Consider extracting the date validation logic into a separate method to maintain a single source of truth and improve maintainability.
+ bool _isValidDateRange(DateTime startDate, DateTime endDate) { + return !endDate.isBefore(startDate); + } + void onStartDate(DateTime startDate) { DateTime endDate = state.endDate; - if (startDate.isAfter(endDate)) { + if (!_isValidDateRange(startDate, endDate)) { endDate = startDate.add(Duration(days: 1)); } // ... } void onEndDate(DateTime endDate) { DateTime startDate = state.startDate; - if (endDate.isBefore(startDate)) { + if (!_isValidDateRange(startDate, endDate)) { startDate = endDate.subtract(Duration(days: 1)); // ... } // ... } void addTournament() async { // ... - if (state.endDate.isBefore(state.startDate)) { + if (!_isValidDateRange(state.startDate, state.endDate)) { state = state.copyWith(showDateError: true, loading: false); return; } // ... }khelo/lib/ui/flow/tournament/match_selection/match_selection_screen.dart (2)
Line range hint
59-71
: LGTM! Consider adding error handling for empty options.Good refactoring to move the match group option logic to the view model. This improves separation of concerns.
Consider handling the case when no options are available:
final option = notifier.getMatchGroupOption(); + if (option.isEmpty) { + context.showErrorSnackBar(context.l10n.no_available_groups_message); + return; + } showActionBottomSheet(
152-156
: Extract magic numbers into named constants.The logic for controlling add button visibility is clear, but the magic numbers should be constants for better maintainability.
Consider extracting the numbers into named constants:
+ const int MAX_QUARTERFINAL_MATCHES = 4; + const int MAX_SEMIFINAL_MATCHES = 2; final showAddButton = - (group == MatchGroup.quarterfinal && matches.length < 4) || - (group == MatchGroup.semifinal && matches.length < 2) || + (group == MatchGroup.quarterfinal && matches.length < MAX_QUARTERFINAL_MATCHES) || + (group == MatchGroup.semifinal && matches.length < MAX_SEMIFINAL_MATCHES) || (group == MatchGroup.finals && matches.isEmpty) || group == MatchGroup.round;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
khelo/lib/ui/flow/home/components/tournament_item.dart
(2 hunks)khelo/lib/ui/flow/tournament/add/add_tournament_screen.dart
(2 hunks)khelo/lib/ui/flow/tournament/add/add_tournament_view_model.dart
(1 hunks)khelo/lib/ui/flow/tournament/detail/components/flexible_space.dart
(1 hunks)khelo/lib/ui/flow/tournament/match_selection/match_scheduler.dart
(1 hunks)khelo/lib/ui/flow/tournament/match_selection/match_selection_screen.dart
(3 hunks)khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart
(1 hunks)style/lib/pickers/date_and_time_picker.dart
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- khelo/lib/ui/flow/tournament/match_selection/match_scheduler.dart
🔇 Additional comments (11)
style/lib/pickers/date_and_time_picker.dart (1)
19-19
: Verify date constraint handling
The logic for firstDate
looks correct, but let's ensure it properly handles all edge cases:
- When
startDate
is afterinitialDate
- When
startDate
is afternow
- When
startDate
is afterlastDate
Consider adding validation to ensure date constraints are properly enforced:
- firstDate: startDate ?? (initialDate.isBefore(now) ? initialDate : now),
+ firstDate: [startDate, initialDate.isBefore(now) ? initialDate : now]
+ .whereType<DateTime>()
+ .reduce((a, b) => a.isBefore(b) ? a : b),
This ensures we always use the earliest valid date as firstDate
.
khelo/lib/ui/flow/tournament/detail/components/flexible_space.dart (2)
28-28
: LGTM: Nullable imageProvider improves error handling
The change to make imageProvider
nullable is appropriate as it better handles cases where tournaments don't have banner images.
35-45
: LGTM: Proper lifecycle management
The lifecycle methods correctly handle image provider initialization and updates. The comparison in didUpdateWidget
ensures the image provider is only reinitialized when the banner URL actually changes.
Let's verify that banner URL changes are properly propagated:
✅ Verification successful
LGTM: Banner URL changes are properly handled
The banner URL is correctly managed across the codebase:
- It's properly defined as a nullable String in the data model
- Changes are handled consistently in both FlexibleSpace and TournamentItem widgets using the same pattern
- The URL is only used for display purposes and proper null checks are in place
- The didUpdateWidget implementation correctly reinitializes the image provider when the URL changes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for places where banner_img_url is modified to ensure proper state updates
rg -l "banner_img_url.*=" --type dart
Length of output: 215
Script:
#!/bin/bash
# Let's examine the actual usage context of banner_img_url in these files
rg "banner_img_url" --type dart -C 3
Length of output: 17971
khelo/lib/ui/flow/home/components/tournament_item.dart (3)
34-34
: LGTM! Improved null safety handling
Making imageProvider
nullable is a good change that prevents potential runtime errors when banner images are not available.
59-65
: LGTM! Well-implemented widget lifecycle handling
The didUpdateWidget
implementation correctly handles banner URL changes and follows widget lifecycle best practices.
Line range hint 34-122
: Verify banner image update fixes across the app
Since this PR aims to fix banner image update issues, let's verify the implementation across related components.
✅ Verification successful
Let me gather more context about the banner image handling in the related files to verify if similar patterns need fixing.
Based on the analysis of the code, I can now provide a final response as the verification is complete.
Banner image update implementation is correctly handled across components
The banner image update implementation in tournament_item.dart
follows the same pattern as other components that handle banner images:
FlexibleSpace
component has identical image update logic withdidUpdateWidget
and_initializeImageProvider
AddTournamentScreen
handles banner updates through state management- The implementation is used consistently in both home screen and view all screen
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar image provider patterns that might need the same fixes
ast-grep --pattern 'ImageProvider$_'
# Look for related banner image handling
rg -l "banner.*image|image.*banner" --type dart
# Check for potential uses of TournamentItem
rg "TournamentItem\(" --type dart
Length of output: 769
Script:
#!/bin/bash
# Check banner image handling in flexible_space.dart
cat "khelo/lib/ui/flow/tournament/detail/components/flexible_space.dart"
# Check banner image handling in add_tournament_screen.dart
cat "khelo/lib/ui/flow/tournament/add/add_tournament_screen.dart"
# Check how TournamentItem is used in home screens
cat "khelo/lib/ui/flow/home/home_screen.dart"
cat "khelo/lib/ui/flow/home/view_all/home_view_all_screen.dart"
Length of output: 34541
khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart (1)
178-183
: Verify finals handling requirements for different tournament types
The current implementation only removes finals if they already exist. Please verify:
- Should finals be allowed in double out format?
- Are there any tournament type specific restrictions for finals similar to quarterfinals and semifinals?
Let's check the codebase for any existing finals-related restrictions:
✅ Verification successful
Based on the code analysis, I can now provide a definitive response regarding the finals handling requirements:
Finals handling is correctly implemented for all tournament types
The code shows that finals handling is consistent with the tournament type requirements:
-
For double out format:
- The code in
match_selection_view_model.dart
shows that finals are handled independently of tournament type, including double out format - This is correct because finals are a valid match group for all tournament types including double out
- The code in
-
Tournament type specific restrictions:
- Unlike quarterfinals and semifinals which have specific restrictions for tournament types (as seen in lines 158-177 of match_selection_view_model.dart)
- Finals are intentionally not restricted by tournament type since they represent the ultimate match(es) in any tournament format
- The only restriction is preventing duplicate finals groups (line 178-183)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for finals-related logic in tournament types
rg -A 5 "MatchGroup.finals.*TournamentType"
# Search for any finals validation logic
ast-grep --pattern 'if ($_ == MatchGroup.finals) { $$$ }'
Length of output: 105
Script:
#!/bin/bash
# Search for finals-related logic in a broader context
rg -A 5 "MatchGroup.finals"
# Search for tournament type validations
rg -A 10 "TournamentType.*match.*group"
# Search for double out format related code
rg "double.*out.*format" -A 5
Length of output: 5662
Script:
#!/bin/bash
# Search for match group enum definition
rg -A 10 "enum MatchGroup"
# Search for tournament type and format related validations
rg -A 10 "TournamentType|TournamentFormat"
# Search for match selection related files
fd "match.*selection.*\.dart$" --exec cat {}
Length of output: 76854
khelo/lib/ui/flow/tournament/match_selection/match_selection_screen.dart (2)
174-181
: LGTM! Clean implementation of the add match button.
The conditional rendering and navigation implementation is clean and passes all necessary parameters correctly.
59-59
: Verify double out format restrictions in view model.
The PR objectives mention preventing quarterfinal/semifinal rounds in "double out" format, but this constraint should be implemented in getMatchGroupOption()
.
Let's verify the implementation:
✅ Verification successful
Let me generate additional scripts to verify the implementation of getMatchGroupOption()
which wasn't found in the previous search:
Double out format restrictions are correctly implemented in view model
The implementation in getMatchGroupOption()
properly handles the double out format restrictions by:
- Removing quarterfinal and semifinal options when tournament type is double out
- Limiting rounds to maximum of 2 for double out format (prevents additional rounds when one already exists)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if getMatchGroupOption properly handles double out format restrictions
# Search for getMatchGroupOption implementation
ast-grep --pattern 'getMatchGroupOption() {
$$$
}'
# Search for double out format related code
rg -i "double.?out" -A 5
Length of output: 7768
Script:
#!/bin/bash
# Search for getMatchGroupOption implementation in match_selection_view_model.dart
cat khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart
Length of output: 6756
khelo/lib/ui/flow/tournament/add/add_tournament_screen.dart (2)
Line range hint 52-60
: LGTM: Well-implemented error handling
The error handling for date validation is well implemented with:
- Proper error observation pattern
- User-friendly error message using localization
- Clean integration with the UI
Line range hint 19-19
: Verify date picker component compatibility
Please ensure that the selectDate
function in style/lib/pickers/date_and_time_picker.dart
supports the new startDate
parameter.
✅ Verification successful
Date picker component supports startDate
parameter
The selectDate
function in style/lib/pickers/date_and_time_picker.dart
already includes support for the startDate
parameter in its signature:
Future<void> selectDate(
BuildContext context, {
String? helpText,
DateTime? startDate, // Parameter exists
required DateTime initialDate,
required Function(DateTime) onDateSelected,
})
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the date picker component supports the startDate parameter
# Search for the selectDate function definition
rg -A 10 "selectDate.*\(" "style/lib/pickers/date_and_time_picker.dart"
Length of output: 326
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
🧹 Outside diff range and nitpick comments (5)
data/lib/service/ball_score/ball_score_service.dart (1)
110-118
: Good implementation of batching, but needs documentation and constants.The chunking implementation effectively solves the Firestore query limitation issue. However, a few improvements would make this more maintainable:
Consider these enhancements:
- Add a constant for the chunk size:
+ /// Maximum number of items allowed in a Firestore whereIn clause + static const int _FIRESTORE_WHERE_IN_LIMIT = 30; Future<List<BallScoreModel>> getBallScoresByMatchIds( List<String> matchIds, ) async { try { if (matchIds.isEmpty) return []; final List<BallScoreModel> data = []; - for (var ids in matchIds.chunked(30)) { + for (var ids in matchIds.chunked(_FIRESTORE_WHERE_IN_LIMIT)) {
- Add documentation explaining the batching behavior:
+ /// Retrieves ball scores for the given match IDs. + /// + /// The method processes match IDs in batches to comply with Firestore's + /// limitation on the number of items in a whereIn clause. + /// Returns an empty list if no match IDs are provided. Future<List<BallScoreModel>> getBallScoresByMatchIds(For future enhancement: Consider implementing pagination if this method needs to handle very large datasets, as loading all scores at once could impact performance.
khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart (2)
145-154
: Improve readability and add documentation.The method could benefit from the following improvements:
- Add documentation to explain the purpose and return value
- Simplify the nested condition for doubleOut tournament type
- Use null-safe operator for better readability
+ /// Returns a list of available match group options based on tournament type and existing matches. + /// Filters out invalid options according to tournament rules. List<MatchGroup> getMatchGroupOption() { final options = MatchGroup.values.toList(); final tournamentType = state.tournament?.type; - if (state.matches.containsKey(MatchGroup.round)) { - if (tournamentType == TournamentType.miniRobin || - (tournamentType == TournamentType.doubleOut && - (state.matches[MatchGroup.round]?.entries.length ?? 0) > 1)) { - options.remove(MatchGroup.round); - } + + final shouldRemoveRound = state.matches.containsKey(MatchGroup.round) && + (tournamentType == TournamentType.miniRobin || + (tournamentType == TournamentType.doubleOut && + state.matches[MatchGroup.round]?.entries.length > 1)); + + if (shouldRemoveRound) { + options.remove(MatchGroup.round); }
172-174
: Improve consistency in group handling.The finals group handling could be made consistent with other group validations by including it in the validation function.
- if (state.matches.containsKey(MatchGroup.finals)) { - options.remove(MatchGroup.finals); - } + removeGroupForRestrictedTypes(MatchGroup.finals);khelo/lib/ui/flow/settings/edit_profile/edit_profile_screen.dart (1)
181-182
: LGTM! Consider extracting date constants.The date range validation is a good addition that prevents invalid birth date selections. However, the hard-coded years could be moved to named constants for better maintainability.
Consider extracting the date values to named constants at the class level:
class EditProfileScreen extends ConsumerWidget { final bool isToCreateAccount; + static final DateTime _minBirthDate = DateTime(1950); const EditProfileScreen({super.key, required this.isToCreateAccount}); final double profileViewHeight = 128;
Then update the selectDate call:
- startDate: DateTime(1950), + startDate: _minBirthDate, endDate: DateTime.now(),khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_overview_tab.dart (1)
130-161
: Consider adding overflow handling for team initialsWhile the layout improvements and text scaling prevention are good, consider adding overflow handling for the team initials text to prevent potential layout issues with very long team names.
Text( initials, style: AppTextStyle.subtitle1.copyWith( color: context.colorScheme.textPrimary, ), + overflow: TextOverflow.ellipsis, ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
data/lib/service/ball_score/ball_score_service.dart
(2 hunks)khelo/assets/locales/app_en.arb
(1 hunks)khelo/lib/components/won_by_message_text.dart
(1 hunks)khelo/lib/ui/flow/settings/edit_profile/edit_profile_screen.dart
(1 hunks)khelo/lib/ui/flow/tournament/detail/components/filter_tab_view.dart
(1 hunks)khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_overview_tab.dart
(5 hunks)khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart
(1 hunks)style/lib/pickers/date_and_time_picker.dart
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- khelo/assets/locales/app_en.arb
🔇 Additional comments (9)
khelo/lib/ui/flow/tournament/detail/components/filter_tab_view.dart (1)
25-30
: LGTM! Good fix for potential text overflow issues.
Wrapping the title text in an Expanded
widget is the correct solution here. It ensures that:
- The title text can utilize available space without causing overflow
- The filter button remains properly aligned and visible
style/lib/pickers/date_and_time_picker.dart (1)
9-10
: LGTM! Parameter additions align with date validation requirements.
The new nullable parameters startDate
and endDate
provide the necessary flexibility for implementing date range validation.
khelo/lib/components/won_by_message_text.dart (2)
27-30
: LGTM! Clear implementation of conditional tie message.
The conditional rendering based on tournament context is well-implemented and maintains consistent styling.
27-30
: Verify localization keys exist.
Let's ensure both localization keys are properly defined.
✅ Verification successful
Both localization keys are properly defined
Both score_board_match_tied
and score_board_match_tied_text
keys are present in khelo/assets/locales/app_en.arb
with appropriate translations:
score_board_match_tied
: "Match tied"score_board_match_tied_text
: "The match has been tied"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if both localization keys exist in the English locale file
# Expected: Both keys should be present in the arb file
# Search for both localization keys in the English locale file
echo "Checking for localization keys in English locale file..."
rg '"score_board_match_tied":' assets/locales/app_en.arb -A 1
rg '"score_board_match_tied_text":' assets/locales/app_en.arb -A 1
Length of output: 387
Script:
#!/bin/bash
# Let's first find the localization files in the repository
fd -e arb
# Then search for the localization keys in all arb files
echo "Searching for localization keys in all arb files..."
rg '"score_board_match_tied":' -A 1
rg '"score_board_match_tied_text":' -A 1
Length of output: 571
khelo/lib/ui/flow/tournament/match_selection/match_selection_view_model.dart (1)
145-177
: Verify implementation matches PR requirements.
The implementation prevents adding quarterfinal/semifinal rounds in double out format as required. However, let's verify that this behavior is consistent across the codebase.
✅ Verification successful
Implementation correctly prevents quarterfinal/semifinal rounds in double out format
The implementation in match_selection_view_model.dart
is consistent with the PR objective and works in conjunction with other files:
- The view model's
handleDoubleOutValidation
method correctly removes quarterfinal/semifinal options for double out tournaments - The match scheduler (
match_scheduler.dart
) handles the group creation logic appropriately - The UI (
match_selection_screen.dart
) respects these restrictions when showing add buttons - No conflicting implementations were found in the codebase
The absence of test files suggests adding tests would be beneficial, but that's outside the scope of verifying the current implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that quarterfinal/semifinal rounds are properly restricted in double out tournaments
# Search for any other places where MatchGroup options are modified
rg -l "MatchGroup\.(quarterfinal|semifinal)"
# Search for tests verifying this behavior
rg -l "test.*double.*out.*quarterfinal" --type-add 'test:*{test,spec}*' --type test
Length of output: 435
Script:
#!/bin/bash
# Let's examine the implementation details in the related files
# Check match_selection_screen.dart for any MatchGroup handling
rg "MatchGroup\.(quarterfinal|semifinal)" khelo/lib/ui/flow/tournament/match_selection/match_selection_screen.dart -B 2 -A 2
# Check match_scheduler.dart for any MatchGroup handling
rg "MatchGroup\.(quarterfinal|semifinal)" khelo/lib/ui/flow/tournament/match_selection/match_scheduler.dart -B 2 -A 2
# Check enum_extensions.dart for any relevant MatchGroup definitions/handling
rg "MatchGroup\.(quarterfinal|semifinal)" khelo/lib/domain/extensions/enum_extensions.dart -B 2 -A 2
# Look for any test files related to match selection or tournament types
fd "test.*match.*selection" --type f
fd "test.*tournament.*type" --type f
Length of output: 2197
khelo/lib/ui/flow/tournament/detail/tabs/tournament_detail_overview_tab.dart (4)
85-110
: LGTM! Layout improvements for match cell view
The changes improve the layout stability by:
- Properly containing WonByMessageText within available space using Expanded
- Adding center alignment for match time display
205-264
: LGTM! Well-structured key stats cell implementation
The implementation shows good attention to detail with:
- Proper text scaling prevention
- Well-organized nested layout
- Appropriate overflow handling for text elements
385-391
: LGTM! Improved info cell layout stability
Good improvements with:
- Proper space allocation using Expanded
- Text scaling prevention for consistent layout
416-424
: LGTM! Better header layout implementation
Good improvements:
- Using Expanded instead of Spacer for better space allocation
- Using Visibility for conditional rendering of the view all button
Changes:
Summary by CodeRabbit
Release Notes
New Features
startDate
andendDate
parameters to the date picker for better user experience.Improvements
Bug Fixes
Refactor