Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds test coverage for timetable UI components and introduces a time selection feature for lecture search, with supporting infrastructure improvements and localization updates.
- Adds comprehensive test coverage for
TimetablePainterand timetable theming logic - Implements interactive time slot selection UI for filtering lecture search results
- Updates project configuration to enable testing and Korean localization
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Project+Templates.swift |
Enables testability in dev builds, adds Korean localization, and updates schemes to include test actions |
InfoPlist.swift |
Configures Korean as development region with bilingual support |
openapi-dev.json |
Updates API spec with diary features, push preferences, and lecture reminders |
TimetableUIComponentsTests.swift |
Removes placeholder test file |
TimetablePainterThemeTests.swift |
Adds comprehensive tests for theme resolution and color assignment logic |
TimetablePainterTests.swift |
Adds tests for coordinate calculations, auto-fit behavior, and manual configuration |
TestHelpers.swift |
Provides stub factories for test data creation |
TimetableGridLayer.swift |
Makes components public and fixes grid line rendering logic |
TimetableBlocksLayer.swift |
Makes component public for test access |
TimetablePainter.swift |
Simplifies empty state detection logic by using aggregatedTimePlaces and removes unused time range selection code |
TimeSelectionSheetViewModelTests.swift |
Adds tests for time selection coordinate conversion and range management |
TimetableScene.swift |
Adds visual separator line to timetable view |
TimeSelectionSheet.swift |
Implements interactive time selection modal for search filtering |
TimeSelectionOverlay.swift |
Implements drag gesture handling for time slot selection |
SearchFilterSheet.swift |
Refactors filter UI to support time selection and improves layout |
LectureSearchResultScene.swift |
Fixes layout structure for predicate display |
EmptyBookmarkView.swift |
Fixes layout to fill available space |
TimeSelectionSheetViewModel.swift |
Implements business logic for time range selection and manipulation |
LectureSearchViewModel.swift |
Adds time filter state management and predicate handling |
LectureSearchAPIRepository.swift |
Removes unused lecture conversion extension |
Localizable.strings (en/Base) |
Adds localized strings for time selection UI |
TimetableSettingView.swift |
Adds proper initialization of timetable data on appear |
TimetableSettingsViewModel.swift |
Refactors initialization to lazy-load timetable data |
CLAUDE.md |
Updates documentation to reflect scheme name changes and testing capabilities |
Comments suppressed due to low confidence (1)
SNUTT/Modules/Feature/Settings/Sources/Presentation/Timetable/TimetableSettingsViewModel.swift:1
- [nitpick] The removal of blank line 13 (between the class declaration and dependency) reduces code readability. Consider keeping blank lines between logically separate sections (dependency declarations, properties, initialization).
import Dependencies
| path.addLine(to: CGPoint(x: containerSize.width, y: 0)) | ||
|
|
||
| for i in 0...hourCount { | ||
| for i in 0..<hourCount { |
There was a problem hiding this comment.
The loop should iterate from 0 to hourCount (inclusive) to draw the final bottom border line. Change 0..<hourCount to 0...hourCount to match the original logic that drew lines for all hour boundaries including the last one.
Suggested change
| for i in 0..<hourCount { | |
| for i in 0...hourCount { |
Comment on lines
+87
to
+90
| func clearAllSelections() { | ||
| clear() | ||
| } | ||
|
|
There was a problem hiding this comment.
The clearAllSelections() method is redundant as it simply delegates to clear(). Either remove clearAllSelections() and use clear() directly, or remove clear() if the public API should only expose clearAllSelections().
Suggested change
| func clearAllSelections() { | |
| clear() | |
| } |
Comment on lines
130
to
136
| func deselectPredicate(predicate: SearchPredicate) { | ||
| selectedPredicates.removeAll(where: { $0 == predicate }) | ||
| if predicate.category == .time { | ||
| selectedPredicates.removeAll(where: { $0.category == .time }) | ||
| } else { | ||
| selectedPredicates.removeAll(where: { $0 == predicate }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing closing brace. The function body is incomplete - add the closing brace after line 135.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
드디어 첫 테스트 코드 추가