Added Deadline field and Deadlines section#155
Conversation
| } | ||
| } | ||
|
|
||
| #Preview { |
There was a problem hiding this comment.
Ниже — по делу, без воды.
-
Несогласованность типов CGFloat/Double:
- @AppStorage("SectionHeight") без явного типа — это Double, а currentSectionHeight — CGFloat. В onEnded вы делаете sectionHeight = currentSectionHeight, что должно падать по типам. Либо явно объявите AppStorage как Double и конвертируйте, либо храните всё как CGFloat с явной конверсией при записи/чтении:
- Рекомендуемо: @AppStorage("SectionHeight") private var sectionHeight: Double = 350 и в коде использовать Double(currentSectionHeight)/CGFloat(sectionHeight).
- @AppStorage("SectionHeight") без явного типа — это Double, а currentSectionHeight — CGFloat. В onEnded вы делаете sectionHeight = currentSectionHeight, что должно падать по типам. Либо явно объявите AppStorage как Double и конвертируйте, либо храните всё как CGFloat с явной конверсией при записи/чтении:
-
Несогласованность дефолтов и ограничений:
- Вы подняли дефолт до 350, но getMaxSectionsHeigth() возвращает 300, если showDeadlinesSection == false. В результате при первом запуске/изменении флага высота может быть больше максимума и не будет «зажата» до первого drag. Нужно клампить currentSectionHeight при onAppear и при изменении showDeadlinesSection.
-
Кламп только в жесте — не хватает реакций на изменения состояния:
- Добавьте кламп в:
- onAppear (после чтения из AppStorage).
- onChange(of: showDeadlinesSection) — если юзер выключил секцию, урежьте currentSectionHeight и сохраните обратно в AppStorage, чтобы не зависать в «некорректном» состоянии.
- Добавьте кламп в:
-
Нейминг и опечатки:
- getMaxSectionsHeigth — опечатка в Height. По гайдлайнам Swift лучше без префикса get: maxSectionHeight (computed property) или maxSectionHeight().
-
Магические числа:
- 50, 300, 350 разбросаны по коду. Соберите в константы (enum Layout { ... }) и используйте единообразно. Так вы избежите расхождений базового максимума (300) и дефолта (350).
-
Безопасность при работе с optional:
- selectedSideBarItem!.name — форс-распаковка в нескольких местах. Перепишите через guard let item = selectedSideBarItem или используйте nil-coalescing. Сейчас любое «мигание» состояния приведёт к крэшу.
-
Инкапсуляция:
- Пометьте внутренние стейты/хранилище как private (например, @AppStorage("showDeadlinesSection") private var showDeadlinesSection) — меньше шанс случайных побочных эффектов при расширении файла.
Предлагаемый фрагмент для правок (идея, не полный файл):
-
Явные типы, константы, кламп на appear/onChange, фиксация типов при записи:
@AppStorage("SectionHeight") private var sectionHeight: Double = 350
@AppStorage("showDeadlinesSection") private var showDeadlinesSection: Bool = true@State private var currentSectionHeight: CGFloat = 350
private enum Layout {
static let minSectionHeight: CGFloat = 50
static let baseMaxSectionHeight: CGFloat = 300
static let deadlinesExtraHeight: CGFloat = 50
}private var maxSectionHeight: CGFloat {
Layout.baseMaxSectionHeight + (showDeadlinesSection ? Layout.deadlinesExtraHeight : 0)
}// В жесте:
currentSectionHeight = min(max(newHeight, Layout.minSectionHeight), maxSectionHeight)// В onEnded:
sectionHeight = Double(min(max(currentSectionHeight, Layout.minSectionHeight), maxSectionHeight))// В onAppear:
.onAppear {
let clamped = min(max(CGFloat(sectionHeight), Layout.minSectionHeight), maxSectionHeight)
currentSectionHeight = clamped
if CGFloat(sectionHeight) != clamped {
sectionHeight = Double(clamped)
}
}// Реакция на изменение флага:
.onChange(of: showDeadlinesSection) { _ in
let clamped = min(max(currentSectionHeight, Layout.minSectionHeight), maxSectionHeight)
if clamped != currentSectionHeight {
currentSectionHeight = clamped
sectionHeight = Double(clamped)
}
} -
И обязательно замените форс-распаковки selectedSideBarItem!.name на безопасный вариант через guard или let item = selectedSideBarItem.
| break | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Коротко по делу:
-
Добавление default: break в switch по enum — сомнительная идея. Это убивает проверку исчерпываемости и прячет ошибки при добавлении новых кейсов в будущем. Если enum ваш (в том же модуле), лучше явно перечислять кейсы и не иметь default вовсе.
-
Если enum из внешней библиотеки/нефрозен и вы осознанно хотите ловить неизвестные будущие кейсы, используйте @unknown default вместо default и хотя бы сигнализируйте в отладке:
switch value {
case .today:
break
case .alltasks:
break
@unknown default:
assertionFailure("Unhandled case: (value)")
break
} -
Сейчас все кейсы делают одно и то же (ничего). Либо уберите весь switch, если он не нужен, либо объедините кейсы:
switch value {
case .today, .alltasks:
break
} -
Если «ничего не делать» — это осознанно, добавьте явный комментарий // no-op или TODO, чтобы не возникало ощущения недореализации.
Итог: либо удалить default, сохранив исчерпываемость, либо заменить на @unknown default с явной диагностикой. Дополнительно сократите дублирование, объединив кейсы или убрав ненужный switch.
| .toggleStyle(.switch) | ||
| } | ||
| case 1: | ||
| Form { |
There was a problem hiding this comment.
Ниже только предметные замечания по диффу и окрестному коду:
-
Доступность: новый @AppStorage("showDeadlinesSection") не помечен как private, в отличие от остальных. Если это состояние не нужно извне — сделайте его private для консистентности и инкапсуляции.
Пример: @AppStorage("showDeadlinesSection") private var showDeadlinesSection = true -
Ключи @AppStorage: сейчас используются строковые литералы. Чтобы исключить опечатки и упростить рефакторинг, вынесите ключи в единое место:
enum SettingsKeys { static let showDeadlinesSection = "showDeadlinesSection"; static let showReviewBadge = "showReviewBadge"; ... }
и используйте @AppStorage(SettingsKeys.showDeadlinesSection) ...
Дополнительно можно зарегистрировать значения по умолчанию через UserDefaults.standard.register(defaults:) в точке входа приложения. -
Нейминг/миграция ключей: свойство showReviewProjectsBadge хранится по ключу "showReviewBadge". Либо привести к единому имени (риск потери пользовательского значения), либо, если решите переименовать ключ, добавьте миграцию (скопировать старое значение в новый ключ один раз). Сейчас расхождение может путать при отладке.
-
UI-деталь: .toggleStyle(.switch)
- На iOS это стиль по умолчанию, модификатор избыточен. Уберите, если не таргетите macOS.
- Если поддерживаете macOS и вам именно свитч, оберните в #if os(macOS) и проверьте доступность API по версии macOS, иначе будет некорректный вид/ворнинги.
-
Локализация: строки в Toggle — пользовательские. Проверьте, что соответствующие ключи есть в Localizable.strings. В SwiftUI строковый литерал — это LocalizedStringKey, но без записанных переводов будет показан английский текст.
-
viewMode как Int: значения 0/1 не самодокументируемые. Лучше ввести enum с RawRepresentable (Int) и использовать его в Picker/segment control. Это снизит риск ошибок при расширении экранов настроек.
Пример:
enum SettingsSection: Int { case general, advanced }
@State private var viewMode: SettingsSection = .general -
Побочный эффект для бейджа: если отключаете showReviewProjectsBadge, обычно нужно сразу сбросить applicationIconBadgeNumber. Если это не делается централизованно в другом месте, добавьте onChange в SettingsView или дерните соответствующий сервис/модель.
Пример (условно):
.onChange(of: showReviewProjectsBadge) { isOn in
#if os(iOS)
if !isOn { UIApplication.shared.applicationIconBadgeNumber = 0 }
#endif
} -
Тест на кроссплатформенность: если SettingsView шарится между iOS/macOS, проверьте, что оба Toggle корректно выглядят и не требуют платформенных специфичных модификаторов (особенно вокруг .toggleStyle и доступности API).
Это минимальные правки, которые улучшат читаемость, консистентность и надежность без изменения поведения.
| Label("Restore defaults", systemImage: "arrow.circlepath") | ||
| } | ||
| } | ||
| .navigationBarTitle("Settings") |
There was a problem hiding this comment.
Критичные и полезные замечания по PR:
-
Неверная иерархия Section/Form:
- Сейчас Section("General") { Form { ... } } и Section("Focus Timer") { Form { ... } }.
- Заголовки секций не будут отображаться, т.к. Section вынесены за пределы Form (Section корректно работает как заголовок только внутри Form/List).
- Также не стоит размещать несколько Form подряд — это даст два независимых списка с лишними отступами/фоном и ухудшит доступность.
- Исправить: один Form, внутри него несколько Section.
-
Устаревший API:
- .navigationBarTitle("Settings") устарел. Используйте .navigationTitle("Settings") (+ при необходимости .navigationBarTitleDisplayMode).
-
Типы данных:
- Количество рабочих сессий (timerWorkSessionsCount) хранится как Double. Это счётчик — его нужно хранить как Int в @AppStorage, а в Picker использовать Int-теги. Это избавит от лишних приведений и потенциальных артефактов округления.
- Для длительностей лучше явно использовать TimeInterval (typealias Double), что повышает читаемость. Можно хранить как Int секунд, но если храните как Double — используйте TimeInterval в сигнатурах и .tag(TimeInterval(...)).
-
Дублирование/жёстко зашитые значения:
- Массив опций минут stride(from: 60, through: 3600, by: 60) повторяется три раза. Вынесите в приватное свойство/константу.
- Значения по умолчанию дублируются в кнопке сброса. Вынесите их в константы/методы (или зарегистрируйте через UserDefaults.register(defaults:) на старте приложения), чтобы единожды определять дефолты.
-
Нейминг/ключи @AppStorage:
- Рассмотрите переименование переменных в более выразительные (например, workDuration, breakDuration, longBreakDuration, sessionsBeforeLongBreak).
- Ключи @AppStorage лучше вынести в enum/namespace, чтобы исключить опечатки.
-
Мелочи:
- .toggleStyle(.switch) внутри Form на iOS избыточен — стиль и так будет switch.
- Если планируется локализация, строковые литералы в SwiftUI уже являются LocalizedStringKey, так что всё ок; просто убедитесь, что ключи добавлены в Localizable.strings.
Предлагаемая правка (структура + типы + устранение дублирования):
struct SettingsView: View {
@AppStorage("workDuration") private var workDuration: TimeInterval = 1500
@AppStorage("breakDuration") private var breakDuration: TimeInterval = 300
@AppStorage("longBreakDuration") private var longBreakDuration: TimeInterval = 1200
@AppStorage("sessionsBeforeLongBreak") private var sessionsBeforeLongBreak: Int = 4
@AppStorage(wrappedValue: true, "showDeadlinesSection") private var showDeadlinesSection: Bool
private let minuteOptions = Array(stride(from: 60, through: 3600, by: 60))
var body: some View {
Form {
Section("General") {
Toggle("Show Deadlines section", isOn: $showDeadlinesSection)
}
Section("Focus Timer") {
Picker("Work session duration", selection: $workDuration) {
ForEach(minuteOptions, id: \.self) { seconds in
Text(Common.formatSecondsToMinutes(Int(seconds)))
.tag(TimeInterval(seconds))
}
}
Picker("Break duration", selection: $breakDuration) {
ForEach(minuteOptions, id: \.self) { seconds in
Text(Common.formatSecondsToMinutes(Int(seconds)))
.tag(TimeInterval(seconds))
}
}
Picker("Long break duration", selection: $longBreakDuration) {
ForEach(minuteOptions, id: \.self) { seconds in
Text(Common.formatSecondsToMinutes(Int(seconds)))
.tag(TimeInterval(seconds))
}
}
Picker("Work sessions before long break", selection: $sessionsBeforeLongBreak) {
ForEach(1...10, id: \.self) { value in
Text("\(value)").tag(value)
}
}
Button {
restoreDefaults()
} label: {
Label("Restore defaults", systemImage: "arrow.circlepath")
}
}
}
.navigationTitle("Settings")
}
private func restoreDefaults() {
workDuration = 1500
breakDuration = 300
longBreakDuration = 1200
sessionsBeforeLongBreak = 4
}
}
Итог: главное — исправить вложенность Section/Form, заменить navigationBarTitle, сделать счётчик Int, вынести опции и дефолты. Это повысит корректность отображения, читаемость и устойчивость к ошибкам.
|
|
||
| HStack { | ||
| if let subtasksCount = task.subtasks?.count, | ||
| subtasksCount > 0, |
There was a problem hiding this comment.
Ниже — только конструктивные замечания по добавленному блоку с дедлайном.
-
Локализация: захардкоженные "Today/Tomorrow/Yesterday" не локализуются. Лучше:
- Либо использовать Date.RelativeFormatStyle с presentation: .named — система сама даст «сегодня/завтра/вчера» на нужном языке.
- Либо вынести строки в Localizable.strings и использовать LocalizedStringKey.
-
Обновление по времени: ваша логика «сегодня/завтра/вчера» и подсветки не будет автоматически обновляться при смене дня/часа, если состояние вью не меняется. Text(style: .relative) обновляется сам, но ваша раскраска — нет. Если хотите корректные обновления без внешних триггеров, оберните в TimelineView (например, раз в минуту) или заведите таймер до следующей полуночи.
-
Гранулярность времени и «просрочено»: сейчас вы сравниваете с startOfDay(for: Date()), из‑за чего задача с дедлайном «сегодня, 09:00» в 15:00 останется «Today» и будет желтой, а не «просрочено» и красной. Если дедлайн содержит время, лучше:
- Просрочено: deadline < Date() (точное время).
- Отображение: для «сегодня» показать время (чтобы понятно, что уже прошло/еще нет).
-
Повторение Calendar.current: дергаете Calendar.current многократно в теле вью. В SwiftUI лучше брать его из окружения и сохранить локально:
@Environment(.calendar) private var calendar
Также можно вычислить now/startOfToday один раз на отрисовку. -
Контраст и семантика цветов: желтый текст часто имеет плохой контраст, особенно в светлой теме. Подумайте о системных семантических вариантах (например, .orange для «скоро», .secondary для иконки) и проверьте доступность.
-
Иконка: "calendar.badge.exclamationmark" показывается всегда. Логичнее ставить её только для просроченных дедлайнов, а для остальных — "calendar" или "calendar.badge.clock".
-
Дублирование модификаторов: .font(.caption) повторяется в каждом ответвлении. Проще применить его один раз к HStack/Group.
-
Структуризация: вынесите логику в отдельный маленький сабвью (DeadlineView) или хотя бы в вспомогательные функции (например, color(for:), text(for:)), чтобы разгрузить TaskDetailsView и облегчить тестирование.
Пример переписывания с учетом локализации, автообновлений и точного статуса (минимально инвазивно):
struct DeadlineView: View {
@Environment(.calendar) private var calendar
let deadline: Date
var body: some View {
TimelineView(.periodic(from: .now, by: 60)) { timeline in
let now = timeline.date
let isOverdue = deadline < now
let isToday = calendar.isDate(deadline, inSameDayAs: now)
HStack {
Image(systemName: isOverdue ? "calendar.badge.exclamationmark" : "calendar")
.foregroundStyle(.secondary)
Group {
if isToday {
// Для «сегодня» показываем время
Text(deadline, format: .dateTime.hour().minute())
} else {
// Локализованные «вчера/завтра/через N дней»
Text(deadline, format: .relative(presentation: .named))
}
}
.foregroundStyle(isOverdue ? Color.red : (isToday ? Color.orange : Color.green))
}
.font(.caption)
}
}
}
Использование:
if let deadline = task.deadline {
DeadlineView(deadline: deadline)
}
Если вместо относительного формата вам нужен именно календарный день для дальних дат, можно заменить .relative на .dateTime.year().month().day() или комбинировать логику выбора формат-стиля в зависимости от расстояния до дедлайна.
|
|
||
| Button { | ||
| deleteItems() | ||
| } label: { |
There was a problem hiding this comment.
Ниже только замечания по делу, без воды.
-
Потенциальный краш из‑за list!. Кнопка теперь скрывается только при list != .deadlines, но это не гарантирует, что list не nil. Если list == nil, кнопка отобразится, а в поповере при передаче list! приложение упадёт. Исправьте условие и избавьтесь от форс‑анврапа.
Пример:- заведите computed var:
- var canCreateTask: Bool { if case let .some(value) = list { return value != .deadlines } else { return false } }
- затем:
- if canCreateTask { … }
- и в popover/передаче в NewTaskView используйте if let list = list { NewTaskView(..., list: list, ...) }.
- заведите computed var:
-
Логика разрешения создания задач захардкожена во вью. Лучше инкапсулировать в модель/enum списка, например свойство allowsCreatingTasks. Тогда UI не будет знать про конкретный .deadlines:
- if list?.allowsCreatingTasks == true { Button(...) … }
-
Switch с default маскирует будущие ошибки. Сейчас .alltasks и default делают одно и то же. Уберите default и перечислите все кейсы явно (или сгруппируйте), чтобы компилятор заставлял добавить обработку при появлении новых значений. Заодно уменьшите дублирование:
- let sorting: (Task, Task) -> Bool
- switch list {
- case .deadlines: sorting = TasksQuery.sortingDeadlines
- case .alltasks, .inbox, .completed: sorting = TasksQuery.sortingWithCompleted
- case nil: sorting = TasksQuery.sortingWithCompleted // если list опционален — решите явно
- innerTasks = tasks.sorted(by: sorting)
-
Именование сортировок: TasksQuery.sortingDeadlines vs sortingWithCompleted выглядят неоднородно. Рассмотрите единый стиль (например, sortingByDeadline, sortingByCompletionThen…).
-
Стабильность и детерминированность сортировки. sorted(by:) не гарантирует стабильность. В обоих компараторах стоит добавить тай‑брейкеры (например, по приоритету, дате создания, id), чтобы одинаковые дедлайны/статусы не «скакали».
-
Обработка nil дедлайна в sortingDeadlines. Убедитесь, что компаратор устойчив к отсутствующим дедлайнам и определите понятный порядок (например, без дедлайна — в конце). Если уже учтено — ок.
-
accessibility: используете .accessibility(identifier:). Для UI‑тестов в SwiftUI корректнее .accessibilityIdentifier("AddToCurrentList"). Текущее API может работать не везде/нестабильно.
-
Платформенные различия: поповер обёрнут в #if os(iOS), а на macOS остаётся только toggle стейта. Проверьте, что где‑то выше/ниже действительно есть .sheet/.popover для macOS, иначе кнопка ничего не показывает. Если нет — добавьте соответствующую презентацию под #if os(macOS).
-
Тестопригодность/горячие клавиши: так как кнопку скрываете для .deadlines, исчезает и shortcut ⌘⌥I. Это ок, если задумано. Если хотели оставить хоткей, но сделать no‑op с подсказкой — вместо скрытия можно disabled(true) с .help(…).
В целом основные риски — форс‑анврап list! и скрытая логика вью. Вынесите правила в доменную модель, уберите force unwrap и сделайте явную исчерпывающую обработку кейсов в switch. Это даст безопасность, предсказуемость и упростит поддержку.
| }, | ||
| "Show estimate" : { | ||
| "localizations" : { | ||
| "ar" : { |
There was a problem hiding this comment.
Ниже — только предметные замечания по добавленным строкам и их переводам.
Терминология и консистентность
- Вы смешали «due date» и «deadline». Если это два разных поля — ок, но тогда в RU нужно явно различать термины. Сейчас: «due date» (ранее в проекте) vs «крайний срок» (новые строки). Если это одно и то же — не вводите «deadline», используйте уже существующую терминологию.
- Внутри RU переводов тоже есть размытие: «Сроки» vs «Крайний срок». Если «Deadlines» — название секции, лучше «Крайние сроки», чтобы сохранялась связь с «Крайний срок» в единственном числе.
- Английский: "Deadline Date" — тавтология. Достаточно "Deadline". Иначе пользователь видит «Deadline Date» рядом с «Due Date» и путается.
- Регистр/стиль в EN-строках не единообразен: у вас уже есть "Set due Date" (неправильная капитализация «Date») и новые "Set Deadline", "Clear deadline" (sentence case vs Title Case). Придерживайтесь одного стиля (для iOS обычно Title Case в заголовках и кнопках):
- Clear Deadline / Clear Due Date
- Set Deadline / Set Due Date
- Deadline
- Deadlines
- Ключи строк совпадают с текстом. Если в проекте уже принято использовать текст как ключ — ок. Но если впереди правки формулировок (очевидны по "Deadline Date"), ключи лучше сделать стабильными (например, settings.show_deadlines_section), а тексты — значениями. Иначе вы блокируете безопасные правки копирайта.
Формулировки RU
- "Очистить крайний срок" — ок, но проверьте консистентность с «Clear due date» (если используется «Очистить дату выполнения»). Лучше одинаковые глаголы: либо «Очистить», либо «Сбросить» везде.
- "Сроки" как перевод "Deadlines" звучит слишком общо. Рекомендую «Крайние сроки», чтобы не терялась связь с единичной формой.
- "Показывать секцию сроков" — калька и звучит неестественно. Лучше: «Показывать раздел «Крайние сроки»» (или «Показывать раздел „Крайние сроки“» — проверьте в проекте стиль кавычек).
- "Deadline Date" -> RU «Крайний срок» уже без слова «дата» — это хорошо. После замены EN на "Deadline" RU менять не нужно.
Контекст и комментарии
- Добавьте developer comments в каталоге строк: где используется каждая строка (кнопка, заголовок секции, лейбл поля, переключатель настройки). Это предотвратит неточные переводы в будущих локалях.
- Clear Deadline — кнопка/действие.
- Deadline — лейбл поля даты.
- Deadlines — заголовок секции.
- Show Deadlines Section — переключатель в настройках.
Покрытие локализаций
- Новые строки добавлены только для en/ru. Если проект поддерживает другие языки (судя по соседним ключам — да), сейчас будет смешанный UI. Стоит сразу добавить хотя бы fallback переводы для уже поддерживаемых локалей или явно решить, что английский — базовый.
Точечные предложения правок
- EN:
- "Deadline Date" -> "Deadline"
- "Clear deadline" -> "Clear Deadline"
- "Show Deadlines section" -> "Show Deadlines Section" или лучше по смыслу настройки: "Show Deadlines" (если в UI уже есть образец «Show estimate» — следуйте принятому шаблону).
- RU:
- "Deadlines" -> "Крайние сроки" (вместо «Сроки»)
- "Show Deadlines section" -> "Показывать раздел «Крайние сроки»"
- Оставить: "Очистить крайний срок", "Установить крайний срок", "Крайний срок"
Возможные коллизии UX
- Если в интерфейсе одновременно есть Due Date и Deadline, предусмотрите пояснение/хинт или различимые контекстные подписи. Иначе пользователи RU не поймут, чем «Дата выполнения» отличается от «Крайнего срока». Если это разные сущности (например, плановая дата vs обязательный дедлайн), добавьте комментарии к переводам и, возможно, вспомогательные строки-подсказки.
Организационное
- Проверьте порядок ключей в .xcstrings: обычно они упорядочены по ключу. Несоблюдение порядка не ломает сборку, но ухудшает поддерживаемость и ревью.
- После правок прогоните экспорт/импорт локализаций (xcodebuild -exportLocalizations / -importLocalizations) — это выявит пропуски и несоответствия схемы.
| var deadline: Date? | ||
|
|
||
| @Relationship(deleteRule: .cascade, inverse: \Todo.parentTask) | ||
| var subtasks: [Todo]? = [Todo]() |
There was a problem hiding this comment.
Ниже — только то, что действительно может укусить в проде или усложнить поддержку.
-
Семантика deadline
- Уточните: это «момент» (точное время) или «дата-день» (без времени)?
- Если это all‑day дедлайн, хранить голый Date опасно: он абсолютный и «поплывет» при смене часового пояса/ДСТ. Лучше:
- хранить DateComponents (год/месяц/день) + timeZone, или
- нормализовать к локальной полуночи и хранить isAllDay + timeZoneId.
- Если это точный момент, оставляйте Date, но убедитесь, что сравнения и сортировки выполняются в UTC и нет логики, завязанной на локаль.
-
Hashable для класса
- Для class Hashable нельзя полагаться на изменяемые поля (deadline, title и т.д.) — при изменении значения ломается инвариант Set/Dictionary.
- Рекомендация: ввести стабильную идентичность и считать hash/== только по ней.
- Добавьте id: UUID и сделайте хеш/равенство по id.
- Отметьте класс final (ускорит диспетчеризацию и сделает семантику очевиднее).
- Если Hashable уже реализован — убедитесь, что deadline в него не входит.
-
Опционалы, где они не нужны
- customRepeatType объявлен как Optional, но имеет дефолт .days — фактически он всегда non‑nil. Сделайте не опциональным:
- var customRepeatType: CustomRepetitionType = .days
- subtasks: [Todo]? = Todo — противоречивое API. Делайте не опциональным с пустым массивом по умолчанию:
- @relationship(...) var subtasks: [Todo] = []
- Это уменьшит количество разворачиваний в коде.
- customRepeatType объявлен как Optional, но имеет дефолт .days — фактически он всегда non‑nil. Сделайте не опциональным:
-
Нейминг/опечатка
- CustomRepeationType — опечатка. Должно быть CustomRepetitionType (или просто RepeatType). Опечатка «утечет» в публичные API/миграции/кей-пафы.
-
Миграции (SwiftData/Core Data)
- Добавление optional Date? — безопасная легковесная миграция. Но:
- Проверьте, что включен автосинтез (для Core Data — опции NSMigratePersistentStoresAutomaticallyOption/NSInferMappingModelAutomaticallyOption).
- В SwiftData убедитесь, что в местах чтения корректно обрабатываете nil для уже существующих записей.
- В предпросмотрах/тестах возможно потребуется пересоздать стор.
- Добавление optional Date? — безопасная легковесная миграция. Но:
-
Отношение subtasks
- Вы уже указали .cascade и inverse: \Todo.parentTask — проверьте, что parentTask существует и не опционал без нужды.
- Если важен порядок подзадач, добавьте явный sortIndex: Int и сортируйте по нему. SwiftData не гарантирует детерминированный порядок массива без явной сортировки.
-
baseTimeHours
- Int «часы» — слабая модель. Нужны ограничения/валидация (0...23?) или более выразительный тип:
- Duration/TimeInterval в секундах или DateComponents(hour:).
- Если это «оценка» времени, лучше хранить в минутах как Int, чтобы избежать дробления.
- Int «часы» — слабая модель. Нужны ограничения/валидация (0...23?) или более выразительный тип:
-
Практика с датами
- Если дедлайн используется для уведомлений/виджетов/сортировок — заведите helper:
- var isOverdue: Bool { guard let d = deadline else { return false }; return d < Date() }
- и централизованную нормализацию даты (Calendar.current.startOfDay(for:)) там, где это уместно.
- Если дедлайн используется для уведомлений/виджетов/сортировок — заведите helper:
Предлагаемые правки (суть, без лишнего):
- final class Todo: Hashable {
-
let id = UUID() -
var baseTimeHours: Int = 0 -
var customRepeatValue: Int = 2 -
var customRepeatType: CustomRepetitionType = .days -
var deadline: Date? -
@Relationship(deleteRule: .cascade, inverse: \Todo.parentTask) -
var subtasks: [Todo] = [] -
static func == (l: Todo, r: Todo) -> Bool { l.id == r.id } -
func hash(into hasher: inout Hasher) { hasher.combine(id) } - }
И отдельно — определитесь с семантикой deadline (момент vs all‑day) и закрепите это в модели данных, чтобы не ловить баги на смене TZ/ДСТ.
| return NSLocalizedString("Deadlines", comment: "") | ||
| case .alltasks: | ||
| return NSLocalizedString("All", comment: "") | ||
| } |
There was a problem hiding this comment.
Ниже — только потенциально проблемные места и улучшения, которые стоит учесть после добавления case .deadlines.
-
Стабильность идентификаторов и обратная совместимость:
- Если id = rawValue (часто делают для Identifiable), зафиксируйте rawValue у всех кейсов явно, чтобы не «поплыли» сохранённые значения при переименованиях:
enum SideBarItem: String { case inbox = "inbox", today = "today", ..., deadlines = "deadlines", alltasks = "alltasks" } - Проверьте места, где rawValue сериализуется (UserDefaults/CoreData/JSON). Для Codable-энумов с rawValue добавление/переименование кейса может ломать декодирование. Рекомендуется кастомный init(from:) с фолбэком:
- При неизвестном значении — подставить, например, .inbox и залогировать событие.
- Если id = rawValue (часто делают для Identifiable), зафиксируйте rawValue у всех кейсов явно, чтобы не «поплыли» сохранённые значения при переименованиях:
-
Локализация: ключи и таблицы:
- Сейчас ключи совпадают с отображаемым текстом ("Deadlines"), что затрудняет перевод и рефакторинг. Лучше использовать стабильные ключи вида "sidebar.deadlines" (и аналогично для остальных) и выделить таблицу, например tableName: "Sidebar".
- Добавьте осмысленный comment для переводчиков, а не пустую строку.
- Для SwiftUI удобнее иметь LocalizedStringKey, чтобы Text(nameKey) не создавалось через String:
var nameKey: LocalizedStringKey { LocalizedStringKey("sidebar.(rawValue)") }
var name: String { NSLocalizedString("sidebar.(rawValue)", tableName: "Sidebar", comment: "Sidebar item title") }
Иначе Text(name) не будет перерассчитываться при смене языка на лету.
-
Порядок отображения:
- CaseIterable.allCases зависит от порядка объявления. Если порядок в сайдбаре важен и может меняться, вынесите его отдельно:
static let displayOrder: [SideBarItem] = [.inbox, .today, .tomorrow, .review, .projects, .deadlines, .alltasks]
- CaseIterable.allCases зависит от порядка объявления. Если порядок в сайдбаре важен и может меняться, вынесите его отдельно:
-
Покрытие switch по всему коду:
- Вы добавили .deadlines в name. Проверьте остальные вычислимые свойства/свитчи (иконка, фильтры, deeplink/роутинг, бейджи и т.д.). Лучше избегать «default», чтобы компилятор подсветил пропущенные места; если «default» нужен, используйте @unknown default и логируйте.
-
Нейминг и консистентность:
- case alltasks vs отображаемое "All" — возможный когнитивный шум. Либо переименуйте кейс в all (при условии сохранения прежнего rawValue "alltasks"), либо отображайте "All Tasks", чтобы соответствовало сущности.
- Убедитесь, что «Deadlines» не дублирует «Today/Tomorrow» с точки зрения фильтра: если это иное измерение (жёсткие due dates), продумайте, как оно будет отличаться в логике фильтрации.
-
Тесты/валидация:
- Юнит‑тест, который бежит по CaseIterable и проверяет, что для каждого кейса есть локализованная строка (не возвращается ключ как значение).
- Снепшот/скриншот‑тест сайдбара для проверки порядка и наличия новых пунктов.
Если нужна конкретика, могу прислать патч с:
- явными rawValue для всех кейсов,
- вынесением ключей локализации в "Sidebar.strings",
- var nameKey для SwiftUI и var name для не-SwiftUI,
- статическим displayOrder.
| } | ||
|
|
||
| static func deleteTask(context: ModelContext, task: Todo) { | ||
| task.disconnectFromAll() |
There was a problem hiding this comment.
Ниже только то, что действительно может быть проблемой, несоответствием или источником багов/долга.
Логика предикатов (даты/границы/согласованность)
- Несогласованность lower bound для «сегодня». В нескольких местах вы проверяете date < tomorrow (и для deadline тоже), но без ограничения >= today. В итоге «сегодня» попадают просроченные задачи (до начала сегодняшнего дня). Если это не задумано — добавьте нижнюю границу: date >= today && date < tomorrow.
- Разная семантика для dueDate vs deadline. Сейчас в большинстве предикатов первым делом проверяется dueDate, а deadline рассматривается как fallback. Если у задачи есть и dueDate, и deadline — ветка deadline никогда не сработает. Проверьте, не хотите ли вы использовать «эффективную дату» как min(dueDate, deadline) во всех фильтрах и сортировках.
- «Сегодня активные» vs «Сегодня». В одном случае вы фильтруете !task.completed, в другом — нет. Убедитесь, что это осознанно. Например, predicateToday сейчас вернёт и незавершённые, и завершённые (включая «просроченные до сегодня» из-за отсутствия lower bound).
- Для completionDate сегодня вы проверяете completeDate >= today && completeDate < tomorrow, но при наличии dueDate логика completionDate даже не выполняется (ранний return). Если цель — «все задачи, завершённые сегодня» плюс задачи, назначенные на сегодня, то такая приоритезация может давать неожиданные результаты (completed «вчера» с dueDate «сегодня» попадут как «сегодня»).
- В предикатах для tomorrow вы корректно задаёте полуинтервал [tomorrow, future), но аналогичную чёткость для today не везде сохраняете.
Сортировка
- Приоритет deadline не учитывается, если dueDate есть только у одного из элементов. В sortingWithCompleted:
- Если у first.dueDate == nil и second.dueDate != nil → возврат false (second идёт раньше). Но может оказаться, что у first.deadline меньше, чем у second.dueDate, и по бизнес‑смыслу first должен идти раньше. Сейчас это невозможно из-за приоритета dueDate над deadline.
- Рекомендация: сравнивать по ключевой дате sortDate = minNonNil(dueDate, deadline) одинаково для обоих элементов. Тогда поведение будет предсказуемым во всех комбинациях.
- Изменение tie‑breaker’а на sortingDeadlines для случая равных dueDate меняет прежнюю семантику (раньше — по priority, теперь — deadline, а уже потом priority). Если это не озвученное требование — можно неожиданно сломать пользовательский порядок.
- Потенциальная нестабильность компаратора. Если в цепочке сравнений всё «равно» (даты равны, deadlines равны, приоритеты равны), сравнение должно давать детерминированный результат. Добавьте финальный tie‑breaker (например, по id) во избежание недетерминированной сортировки.
- Не используйте принудительные развёртки (!) в сортировке. Сейчас они защищены проверками, но безопаснее и чище сравнивать опционалы без force unwrap:
- Схема: сравнивать пары (optA, optB) через свитч либо сопоставляя nil с .distantFuture.
Пример упрощённой и согласованной сортировки
- Общая идея: единый ключ сортировки по дате = dueDate ?? deadline, затем priority.
- Сигнал завершенности можно учитывать первым критерием (незавершённые раньше завершённых).
Пример:
static func sortKeyDate(_ t: Todo) -> Date {
(t.dueDate ?? t.deadline) ?? .distantFuture
}
static func sortingWithCompleted(_ a: Todo, _ b: Todo) -> Bool {
if a.completed != b.completed {
return b.completed // незавершённые идут раньше
? true
: false
}
let ad = sortKeyDate(a)
let bd = sortKeyDate(b)
if ad != bd { return ad < bd }
if a.priority != b.priority { return a.priority > b.priority }
return a.id.uuidString < b.id.uuidString // стабильный tie‑breaker
}
Единообразие предикатов для dueDate и deadline
- Сейчас в каждом предикате дублируется логика: сначала dueDate, потом deadline. Если бизнес‑правило «ориентируемся на min(dueDate, deadline)», то предикаты должны отражать это правило. Для #Predicate нельзя выносить произвольные хелперы, но можно явно записать логическое ИЛИ, эквивалентное min(dueDate, deadline):
// Пример "сегодня активные" по min(due, deadline):
return #Predicate { task in
let inTodayDue = (task.dueDate != nil) && task.dueDate! >= today && task.dueDate! < tomorrow
let inTodayDeadlineWhenNoDue = (task.dueDate == nil) && (task.deadline != nil) && task.deadline! >= today && task.deadline! < tomorrow
return (inTodayDue || inTodayDeadlineWhenNoDue) && !task.completed
}
- Аналогично для tomorrow и для «всё до завтра» и т.п. Это убирает логические расхождения между dueDate и deadline.
Работа с «сегодня/завтра»
- Инициализируйте today/tomorrow/future один раз на входе функции и используйте одинаково для dueDate и deadline во всех ветках.
- Для тестируемости и предсказуемости можно принимать «now»/Calendar через параметры (по умолчанию Date()/Calendar.current), а в тестах подставлять фиктивные значения. Это защитит от ошибки «предикат построен до полуночи, использован после».
Нейминг/интерфейс
- predicateDeadlines и predicateDeadlinesActive — ок, но проверьте, не пересекаются ли они по смыслу с уже существующими списками «сегодня/завтра/активные» и не дублируют ли выборки.
- Вы удалили predicateTodayAssign и ввели новый predicateTomorrow раньше по файлу. Убедитесь, что нигде не осталось вызовов старого имени. Имена старайтесь делать симметричными: today/todayActive, tomorrow/tomorrowActive, deadlines/deadlinesActive.
Производительность/индексы
- Если это SwiftData/Core Data с #Predicate, запросы вида (dueDate условие) OR (deadline условие) могут хуже использовать индексы. Если таких выборок много и данных много, имеет смысл проиндексировать оба поля и completed.
- Избегайте слишком частого пересоздания предикатов на каждый ререндер — кэшируйте при необходимости (но не переживайте «смену дня»: пересоздавайте при смене суток).
Итого, что лучше поправить прямо сейчас:
- Определиться с единым правилом: используем min(dueDate, deadline) для фильтров и сортировок или нет. Если да — переписать предикаты/сортировку под эту концепцию.
- Добавить нижнюю границу today (>= today) там, где нужна именно «сегодня», а не «до завтра».
- Унифицировать «active» предикаты: везде одинаково учитывать !task.completed и для dueDate, и для deadline.
- Сделать сортировку детерминированной (финальный tie‑breaker), убрать force unwrap в сравнении дат, учесть deadline даже когда dueDate есть только у одного из элементов.
| } | ||
| } | ||
|
|
||
| #Preview { |
There was a problem hiding this comment.
Ниже только замечания по реальным рискам и улучшениям, без тривиального:
-
Несоответствие типов @AppStorage и CGFloat
- Сейчас sectionHeight (Double по инференсу из 350.0) присваивается из currentSectionHeight (CGFloat) и наоборот. Это либо не компилируется, либо потенциально ломко.
- Явно укажите тип и делайте конвертации:
- @AppStorage("SectionHeight") private var sectionHeight: Double = 350
- В onAppear: currentSectionHeight = CGFloat(sectionHeight)
- В onEnded: sectionHeight = Double(currentSectionHeight)
-
Ошибка в логике DragGesture: «двойной учёт» translation
- Вы добавляете gesture.translation.height к уже изменённому currentSectionHeight, из-за чего высота «убегает».
- Правильнее брать «базовую» высоту на старт жеста. Проще всего использовать sectionHeight как базу:
- onChanged { value in currentSectionHeight = clamp(CGFloat(sectionHeight) + value.translation.height) }
- onEnded { _ in sectionHeight = Double(currentSectionHeight) }
- Где clamp применяет min/max-ограничения.
-
Несогласованность дефолтов и максимума
- По умолчанию current/sectionHeight = 350, а getMaxSectionsHeight возвращает 300 или 350 в зависимости от showDeadlinesSection. Если showDeadlinesSection == false, дефолт уже превышает максимум и будет «прыжок» при первом перетаскивании.
- Решение:
- Привести дефолт в соответствие макс-значению (например, хранить в @AppStorage последний валидный размер и клампить его при старте).
- На onAppear и при изменении showDeadlinesSection явно клампить currentSectionHeight и синхронизировать sectionHeight:
- onAppear { currentSectionHeight = clamp(CGFloat(sectionHeight)) }
- onChange(of: showDeadlinesSection) { _ in
currentSectionHeight = clamp(currentSectionHeight)
sectionHeight = Double(currentSectionHeight)
}
-
Избегайте force unwrap selectedSideBarItem!
- В кейсах вы делаете title: selectedSideBarItem!.name. Это краш при nil.
- Либо оборачивайте switch в if let item = selectedSideBarItem { ... }, либо подставляйте безопасное значение: title: selectedSideBarItem?.name ?? "".
-
Нейминг и «магические числа»
- getMaxSectionsHeight: в Swift не принято префикс «get». Лучше maxSectionHeight. «Sections» во множественном числе выглядит странно.
- Вынесите константы в локальные Metrics, чтобы не плодить магические числа (50, 300, 350) по коду и держать их в одном месте:
- enum Metrics { static let minSectionHeight: CGFloat = 50; static let baseMax: CGFloat = 300; static let deadlinesExtra: CGFloat = 50 }
- var maxSectionHeight: CGFloat { Metrics.baseMax + (showDeadlinesSection ? Metrics.deadlinesExtra : 0) }
-
Клампинг — один путь
- Вместо дублирования min(max(...)) в нескольких местах, добавьте маленькую функцию:
- func clamp(_ h: CGFloat) -> CGFloat { min(max(h, Metrics.minSectionHeight), maxSectionHeight) }
- Используйте её в onChanged, onAppear и onChange(of: showDeadlinesSection).
- Вместо дублирования min(max(...)) в нескольких местах, добавьте маленькую функцию:
-
Повтор .environmentObject
- Вы трижды повторяете .environmentObject(showInspector) и .environmentObject(selectedTasks) в разных кейсах. Это легко забыть при добавлении нового кейса.
- Вынесите обёртку выше (на общий контейнер после switch) или заверните контент кейса во вспомогательный view, куда один раз повесить environmentObject.
-
Логическая консистентность showDeadlinesSection
- Сейчас флаг влияет только на максимум высоты. Если секция «Сроки» скрыта в сайдбаре, но остаётся доступной через selectedSideBarItem == .deadlines, это может дать неожиданный UI. Убедитесь, что навигация не позволяет попасть в скрытый раздел (или корректно обрабатывайте такой кейс).
-
Мелкие замечания
- @query var tasks: [Todo] не используется — удалить, если действительно не нужно.
- Ключи @AppStorage лучше централизовать (enum AppStorageKey { static let sectionHeight = "SectionHeight"; static let showDeadlinesSection = "showDeadlinesSection" }), чтобы избежать опечаток.
Резюмируя: главное — исправить типизацию @AppStorage/CGFloat, починить математику жеста (использовать фиксированную базу), синхронизировать дефолты и клампинг с флагом showDeadlinesSection, убрать force unwrap и разнести «магические числа» в понятные константы. Это устранит потенциальные краши, «уплывание» размера и неожиданные скачки высоты.
| Label("Restore defaults", systemImage: "arrow.circlepath") | ||
| } | ||
| } | ||
| .navigationBarTitle("Settings") |
There was a problem hiding this comment.
Ниже только предметные замечания и улучшения.
-
Типы значений. Для дискретных настроек лучше Int вместо Double:
- секунды и количество сессий — целые значения; Double в Picker + .tag(Double(...)) добавляет лишнюю неточность и бойлерплейт.
- Рекомендация: хранить в @AppStorage секунды как Int и работать в UI в минутах через привязки-конвертеры.
Пример:- @AppStorage("timerWorkSessionSec") private var workSec: Int = 1500
- private var workMin: Binding { .init(get: { workSec / 60 }, set: { workSec = $0 * 60 }) }
Аналогично для break/long break. Для количества сессий — @AppStorage Int и Picker c Int тегами.
-
Диапазоны для Picker. Сейчас вы каждый раз создаете Array(stride(...)). Это лишние аллокации и шум.
- Если переходите на минуты, используйте 1...60 (RandomAccessCollection, без Array(stride)).
- Если оставляете секунды — заведите один общий массив опций или генератор и переиспользуйте.
-
Дублируемая разметка Picker-ов. Три почти одинаковых Picker-а — сигнал к выносу в переиспользуемый подвид/функцию для снижения ошибок и упрощения изменений.
- Например, маленький DurationPicker(title:selection:options:formatter:).
-
Форматирование длительностей. Common.formatSecondsToMinutes, скорее всего, выдает строку руками. Для локализации и правильных единиц лучше использовать DateComponentsFormatter/MeasurementFormatter. Тогда исчезнут ручные склонения/множественное число и это будет консистентно везде.
-
Ключи и значения по умолчанию. Магические строки/числа лучше централизовать:
- enum SettingsKey { static let workSec = "timerWorkSessionSec" ... }
- struct Defaults { static let workSec = 1500; ... }
- Кнопка “Restore defaults” пусть берет значения из Defaults, а не из захардкоженных чисел.
-
Согласованность области видимости. Остальные @AppStorage помечены private, новый showDeadlinesSection — нет. Приведите к одному стилю (private), если свойство не нужно извне.
-
Обновление заголовка навигации. .navigationBarTitle("Settings") устарел. Используйте .navigationTitle("Settings") (+ при необходимости .navigationBarTitleDisplayMode).
-
UX стиля Picker-ов. В iOS 16+ внутри Form удобнее .pickerStyle(.navigationLink), чтобы не выпадал wheel/inline там, где не ожидается, и опции открывались отдельным экраном.
-
“Restore defaults”. Чтобы визуально “подтвердить” изменения, можно обернуть присваивания в withAnimation. Плюс, логика сброса пусть будет в отдельном методе resetToDefaults(), чтобы не плодить дубликаты при добавлении новых настроек.
-
Валидация/ограничения (по желанию). Можно гарантировать согласованность значений:
- например, long break ≥ break; рабочая сессия ≥ 5 мин, и т.д. Сделайте onChange для биндингов или валидацию при установке.
Итоговая минимальная правка-скелет (идея):
- Хранить секунды как Int в @AppStorage.
- В UI работать в минутах через Binding-конвертеры и диапазоны 1...60.
- Заменить navigationBarTitle на navigationTitle.
- Вынести ключи/дефолты в константы и вытащить общую часть Picker-а.
| } | ||
| case .settings: | ||
| NavigationLink(value: item) { | ||
| HStack { |
There was a problem hiding this comment.
Ниже только по делу — о реальных рисках и улучшениях в контексте добавления @AppStorage и условного показа .deadlines.
- Не прячьте ряд через EmptyView внутри List/ForEach
- Сейчас вы итерируете List(SideBarItem.allCases, ...) и для .deadlines отдаёте EmptyView, если флаг выключен. Это может приводить к:
- “пустым” строкам/глитчам при диффинге List,
- несоответствию selection (выбран .deadlines, а строки нет),
- более сложной логике диффа/анимаций.
- Вместо этого фильтруйте источник данных ДО List:
- Пример:
let items = SideBarItem.allCases.filter { $0 != .deadlines || showDeadlinesSection }
List(items, selection: $selectedSideBarItem) { item in
switch item {
case .deadlines:
NavigationLink(value: item) { /* label */ }
...
}
}
- Пример:
- Синхронизируйте selection при скрытии .deadlines
- Если .deadlines выбран и вы выключаете showDeadlinesSection, то selection станет невалидным.
- Добавьте onChange:
.onChange(of: showDeadlinesSection) { newValue in
if !newValue, selectedSideBarItem == .deadlines {
selectedSideBarItem = .inbox /* или другой безопасный дефолт */
}
}
- Улучшите доступность и тестопригодность
- Добавьте идентификатор для дедлайнов:
.accessibilityIdentifier("DeadlinesNavButton") - Строку "Deadlines" вынесите в локализацию (например, LocalizedStringKey "deadlines_title"), чтобы не хардкодить.
- Упростите разметку лейбла и аккуратнее с цветами
- Вместо HStack + Image + Text используйте Label — короче и консистентнее:
NavigationLink(value: item) {
Label("deadlines_title", systemImage: "calendar.badge.exclamationmark")
} - Сейчас вы красите весь контент .foregroundStyle на HStack. Это может конфликтовать с системными цветами выделения в sidebar. Рекомендации:
- Если хотите цветной только символ — примените стиль только к Image, текст оставьте системным.
- Лучше использовать semantic цвета из ассетов:
Color("SidebarDeadlines") с light/dark вариантами вместо #colorLiteral.
- Scope и ключ для @AppStorage
- Сузьте область видимости:
@AppStorage(AppStorageKey.showDeadlinesSection) private var showDeadlinesSection = true
enum AppStorageKey { static let showDeadlinesSection = "settings.showDeadlinesSection" } - Это уменьшит риск коллизий ключей и улучшит читаемость. Если настройки предполагаются общими между таргетами — рассмотрите store: UserDefaults(suiteName: ...).
- Совместимость SFSymbol
- calendar.badge.exclamationmark доступен не на всех системах. Если поддерживаете старые версии — добавьте fallback символ или условную компиляцию/доступность.
Итог: ключевые изменения — фильтровать список элементов до List (без EmptyView ветки) и сбрасывать selection при скрытии секции. Плюсом — локализация строки, аккуратная окраска и неймспейс для @AppStorage. Это устранит потенциальные баги и сделает код устойчивее.
No description provided.