chore: navigation rework#152
Conversation
WalkthroughRoute navigation now matches by route path or by an array of route names; page meta names and canReturn flags were added. Several flow components received motion/typography tweaks. Task store gained user- and today-based computed lists; the daily report title was prefixed with "Задачи ". Public NavigationRoute type changed name → names: string[]. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router
participant NavigationButton
participant useNavigation as Navigation composable
Note over Router,NavigationButton: Pages register meta via definePageMeta(name, canReturn?)
User->>Router: navigate to path (sets currentRoute { path, name, meta })
Router-->>NavigationButton: currentRoute
rect rgb(220,235,255)
NavigationButton->>NavigationButton: isThisRoute = match path (and exact?)
NavigationButton->>useNavigation: read mainRoutes (each has `names: []`)
NavigationButton->>NavigationButton: isThisName = currentRoute.name ∈ route.names
end
alt active (isThisRoute or isThisName)
NavigationButton-->>User: render active styling (tg-text, active classes)
else inactive
NavigationButton-->>User: render default styling
end
opt return available
Note right of NavigationButton: meta.canReturn && isThisName
NavigationButton-->>User: show undo icon (return action)
end
opt scroll-to-top
NavigationButton-->>User: show up-arrow when canScrollToTop
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/atrium-telegram/shared/types/index.ts (1)
15-21: Inconsistent NavigationRoute API — storefront-telegram still exposesname: string(breaking).
- apps/atrium-telegram/shared/types/index.ts →
names: string[]and its route defs usenames.- apps/storefront-telegram/shared/types/index.ts still defines
name: string— change tonames: string[]and migrate storefront consumers (e.g. apps/storefront-telegram/app/composables/useNavigation.ts, apps/storefront-telegram/app/components/NavigationButton.vue) to read/writeroute.names.- This is a breaking public type change; align types across packages or revert until both are migrated.
🧹 Nitpick comments (9)
apps/atrium-telegram/app/components/flow/OrdersOnline.vue (1)
2-2: Respect reduced-motion preferences for new animationsIf your motion preset doesn’t automatically honor prefers-reduced-motion, consider gating with a motion-safe variant or disabling on reduce to avoid accessibility regressions.
apps/atrium-telegram/app/pages/index.vue (1)
14-21: Prefer named-route navigation for resilienceLinking by name survives path changes and keeps params typed.
Apply:
- :to="`/flow/${item.id}`" + :to="{ name: 'flow-itemId', params: { itemId: item.id } }"apps/atrium-telegram/app/pages/epic/[epicId]/index.vue (1)
26-30: Condition always true; hide comments when list is emptylength >= 0 is always true. Use > 0 to avoid showing a “0 comments” pill.
Apply:
- <div v-if="epic?.comments && epic?.comments.length >= 0" class="flex flex-row items-center gap-2"> + <div v-if="epic?.comments && epic?.comments.length > 0" class="flex flex-row items-center gap-2">apps/atrium-telegram/app/components/flow/ItemCard.vue (1)
9-11: Guard comments length accessIf comments can be undefined, this will throw. Safer to chain and default to 0.
Apply:
- <div class="w-full text-base/5 font-normal whitespace-pre-wrap break-words line-clamp-12"> + <div class="w-full text-base/5 font-normal whitespace-pre-wrap break-words line-clamp-12"> {{ item.description }} </div> @@ - <p>{{ item?.comments.length }}</p> + <p>{{ item?.comments?.length ?? 0 }}</p>Also applies to: 16-18
apps/atrium-telegram/app/components/flow/OrderAmountAverage.vue (1)
2-2: Add reduced-motion consideration to new animationEnsure the preset respects prefers-reduced-motion or gate it to avoid motion for users who opt out.
apps/atrium-telegram/app/components/flow/FeedbackAverage.vue (1)
2-2: Respect reduced-motion for animation presetSame advice: gate or auto-respect prefers-reduced-motion to prevent unnecessary motion.
apps/atrium-telegram/app/components/flow/KitchensOnline.vue (1)
2-2: Apply motion with reduced-motion safety on both branchesGood consistency adding presets to both states; add motion-safe handling if the preset doesn’t already.
Also applies to: 23-23
apps/web-app/server/tasks/ai/daily-report.ts (1)
60-61: Avoid hardcoding the Russian title; prefer config/i18n.Hardcoding "Задачи" limits flexibility. Consider pulling a title prefix from runtime config or an i18n key so environments can localize without code changes.
apps/atrium-telegram/shared/types/index.ts (1)
17-17: Make route names immutable to prevent accidental mutation.Declare names as readonly for safer typing.
- names: string[] + names: readonly string[]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/atrium-telegram/app/components/NavigationButton.vue(4 hunks)apps/atrium-telegram/app/components/flow/FeedbackAverage.vue(1 hunks)apps/atrium-telegram/app/components/flow/ItemCard.vue(1 hunks)apps/atrium-telegram/app/components/flow/KitchensOnline.vue(2 hunks)apps/atrium-telegram/app/components/flow/OrderAmountAverage.vue(1 hunks)apps/atrium-telegram/app/components/flow/OrdersOnline.vue(1 hunks)apps/atrium-telegram/app/composables/useNavigation.ts(1 hunks)apps/atrium-telegram/app/pages/epic/[epicId]/index.vue(1 hunks)apps/atrium-telegram/app/pages/flow/[itemId]/index.vue(2 hunks)apps/atrium-telegram/app/pages/index.vue(2 hunks)apps/atrium-telegram/shared/types/index.ts(1 hunks)apps/web-app/server/tasks/ai/daily-report.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
apps/atrium-telegram/app/pages/index.vue (2)
13-13: UI spacing change looks goodThe increased gap improves scan-ability.
27-29: Avoid duplicating router name in meta; prefer route.nameRan: rg -n 'currentRoute.value.meta.name|route.meta.name|isFlowInnerPage|canReturnToMain' — no matches found in the repo. Absence of matches isn't definitive; confirm there are no template/runtime or external accesses to meta.name. If none exist, remove meta.name and read router.currentRoute.value.name for active checks; keep meta only for canReturn.
apps/atrium-telegram/app/pages/epic/[epicId]/index.vue (1)
62-65: Page meta aligns with navigation schemeMeta name and canReturn fit the new names-based navigation.
apps/atrium-telegram/app/components/flow/ItemCard.vue (1)
5-7: Typography tweak looks goodBigger title improves hierarchy.
apps/atrium-telegram/app/pages/flow/[itemId]/index.vue (2)
8-10: Heading scale change is fineImproves readability on detail view.
20-23: Meta name + canReturn: OK — active state uses route.name
definePageMeta name 'flow-itemId' and useRoute('flow-itemId') are present (apps/atrium-telegram/app/pages/flow/[itemId]/index.vue) and NavigationButton checks router.currentRoute.value.name via route.names.includes(...) (apps/atrium-telegram/app/components/NavigationButton.vue). No change required.apps/web-app/server/tasks/ai/daily-report.ts (1)
56-61: Timezone/year formatting: reduce off-by-one and ambiguity.The report date uses server time and omits the year. Near midnight or across year boundaries, titles can be misleading. Consider formatting with year and, if relevant, a fixed business timezone.
Example outside this hunk:
// Top: import { utcToZonedTime } from 'date-fns-tz' const tz = process.env.TZ ?? 'Europe/Moscow' const now = utcToZonedTime(new Date(), tz) const date = format(now, 'd MMMM yyyy', { locale: ru })apps/atrium-telegram/app/composables/useNavigation.ts (2)
9-23: LGTM on route-name arrays; aligns with meta-based navigation.
6-26: Verify page meta names exist for all included entries.Ripgrep returned no hits under apps/atrium-telegram/app/pages; confirm each page referenced by useNavigation (apps/atrium-telegram/app/composables/useNavigation.ts:6-26) declares definePageMeta({ name: '...' }) for: index, flow-itemId, epic, epic-epicId, tasks.
apps/atrium-telegram/app/components/NavigationButton.vue (2)
9-10: Active styling via name-match looks good.Also applies to: 31-32
18-21: Return icon condition depends on route.meta.canReturn — verification neededFile: apps/atrium-telegram/app/components/NavigationButton.vue (lines 18–21). Search for pages setting definePageMeta({ canReturn: true }) under apps/atrium-telegram/app/pages returned no matches; confirm inner pages set meta.canReturn where a return icon is expected or update the component to handle missing meta.
| const router = useRouter() | ||
|
|
||
| const isThisRoute = computed(() => route.exact ? router.currentRoute.value.path === route.path : router.currentRoute.value.path.startsWith(route.path)) | ||
| const isThisName = computed(() => route.names.includes(router.currentRoute.value.name)) |
There was a problem hiding this comment.
Type-safety fix: route.name can be RouteRecordName | null.
Guard before includes to avoid TS errors and edge cases.
-const isThisName = computed(() => route.names.includes(router.currentRoute.value.name))
+const isThisName = computed(() => {
+ const n = router.currentRoute.value.name
+ return typeof n === 'string' && route.names.includes(n)
+})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isThisName = computed(() => route.names.includes(router.currentRoute.value.name)) | |
| const isThisName = computed(() => { | |
| const n = router.currentRoute.value.name | |
| return typeof n === 'string' && route.names.includes(n) | |
| }) |
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/components/NavigationButton.vue around line 47, the
computed uses route.names.includes(router.currentRoute.value.name) but
router.currentRoute.value.name can be RouteRecordName | null; guard against null
before calling includes by first reading router.currentRoute.value.name into a
local const, check it is not null (or is a string/RouteRecordName) and only then
call route.names.includes with the narrowed value so TypeScript is satisfied and
runtime edge cases are avoided.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/atrium-telegram/app/components/NavigationButton.vue (1)
56-56: Type-safety: guard currentRoute.name before includesSame note as earlier review: router.currentRoute.value.name can be RouteRecordName | null. Narrow before includes.
-const isThisName = computed(() => route.names.includes(router.currentRoute.value.name)) +const isThisName = computed(() => { + const n = router.currentRoute.value.name + return typeof n === 'string' && route.names.includes(n) +})
🧹 Nitpick comments (2)
apps/atrium-telegram/shared/types/index.ts (1)
17-22: Make route names immutable; keep badge optional but clarify zero-value behavior
- Prefer a readonly array to avoid accidental mutation and reduce reactive churn.
- Badge: upstream sets it from task count; make sure zero doesn’t render a chip (addressed in useNavigation/NavigationButton comments).
export type NavigationRoute = { path: string - names: string[] + names: readonly string[] title: string icon: string exact?: boolean badge?: string }apps/atrium-telegram/app/pages/tasks/index.vue (1)
41-45: Guard against undefined user id when passing current-user-idThere’s an explicit cast to string. If userStore.id isn’t populated yet, TaskList may receive an invalid id.
- <TaskList - v-for="taskList in taskStore.myLists" - :key="taskList.id" - :list-id="taskList.id" - :current-user-id="userStore.id as string" - /> + <template v-if="userStore.id"> + <TaskList + v-for="taskList in taskStore.myLists" + :key="taskList.id" + :list-id="taskList.id" + :current-user-id="userStore.id" + /> + </template>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/atrium-telegram/app/components/NavigationButton.vue(3 hunks)apps/atrium-telegram/app/composables/useNavigation.ts(1 hunks)apps/atrium-telegram/app/pages/tasks/index.vue(3 hunks)apps/atrium-telegram/app/stores/task.ts(3 hunks)apps/atrium-telegram/shared/types/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/atrium-telegram/app/stores/task.ts (2)
apps/atrium-telegram/app/stores/user.ts (1)
useUserStore(8-135)packages/database/src/repository/task.ts (1)
lists(44-68)
apps/atrium-telegram/app/composables/useNavigation.ts (2)
apps/atrium-telegram/app/stores/task.ts (1)
useTaskStore(14-102)apps/atrium-telegram/shared/types/index.ts (1)
NavigationRoute(15-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
apps/atrium-telegram/app/stores/task.ts (2)
2-2: Good choice of @internationalized/date for TZ-safe “today” checksLGTM.
95-97: Exposing computed APIs from the storeExporting myLists/myTodayTasks from the store looks good and centralizes derivations.
apps/atrium-telegram/app/pages/tasks/index.vue (1)
15-26: Header copy/pluralization wired to store — looks goodBinding to taskStore.myTodayTasks.length and pluralization is correct.
apps/atrium-telegram/app/composables/useNavigation.ts (2)
6-7: Store usage inside shared composableUsing taskStore inside _useNavigation is fine and keeps badge reactive.
11-19: Verify route name arrays match actual record namesEnsure ['index','flow-itemId'] and ['epic','epic-epicId'] exactly match router.currentRoute.value.name values; otherwise isThisName will never activate.
Would you like a quick repo scan script to list discovered route names from pages/ to double-check?
apps/atrium-telegram/app/components/NavigationButton.vue (2)
9-10: Active styling by route or nameBroadening active state to include route name is correct.
18-21: Undo icon gating via meta.canReturn + name is reasonableAssuming pages set meta.canReturn when appropriate.
| <UChip | ||
| v-else | ||
| :name="route.icon" | ||
| class="size-6 motion-preset-shake" | ||
| /> | ||
| size="3xl" | ||
| :show="!!route.badge" | ||
| :text="route.badge" | ||
| :ui="{ | ||
| base: '-right-1 px-1.5 py-2 ring-2 tg-text-button font-bold motion-translate-y-loop-25 motion-duration-3500', | ||
| }" | ||
| > | ||
| <UIcon | ||
| :name="route.icon" | ||
| class="size-6 motion-preset-shake" | ||
| /> | ||
| </UChip> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t show chip when badge is '0'
Guard with a numeric check to avoid rendering a “0” badge.
- <UChip
- v-else
- size="3xl"
- :show="!!route.badge"
- :text="route.badge"
+ <UChip
+ v-else
+ size="3xl"
+ :show="Number(route.badge) > 0"
+ :text="route.badge"
:ui="{
base: '-right-1 px-1.5 py-2 ring-2 tg-text-button font-bold motion-translate-y-loop-25 motion-duration-3500',
}"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <UChip | |
| v-else | |
| :name="route.icon" | |
| class="size-6 motion-preset-shake" | |
| /> | |
| size="3xl" | |
| :show="!!route.badge" | |
| :text="route.badge" | |
| :ui="{ | |
| base: '-right-1 px-1.5 py-2 ring-2 tg-text-button font-bold motion-translate-y-loop-25 motion-duration-3500', | |
| }" | |
| > | |
| <UIcon | |
| :name="route.icon" | |
| class="size-6 motion-preset-shake" | |
| /> | |
| </UChip> | |
| <UChip | |
| v-else | |
| size="3xl" | |
| :show="Number(route.badge) > 0" | |
| :text="route.badge" | |
| :ui="{ | |
| base: '-right-1 px-1.5 py-2 ring-2 tg-text-button font-bold motion-translate-y-loop-25 motion-duration-3500', | |
| }" | |
| > | |
| <UIcon | |
| :name="route.icon" | |
| class="size-6 motion-preset-shake" | |
| /> | |
| </UChip> |
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/components/NavigationButton.vue around lines 22–35,
the UChip currently uses :show="!!route.badge" which causes a "0" badge to
render; change the show binding to a numeric check like
:show="Number(route.badge) > 0" (or use parseInt and isFinite if you expect
non-numeric values) so the chip only renders when the badge value is a positive
number.
| path: '/tasks', | ||
| name: 'tasks', | ||
| names: ['tasks'], | ||
| title: t('app.my-tasks'), | ||
| icon: 'i-lucide-layout-dashboard', | ||
| badge: taskStore.myTodayTasks.length.toString(), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid showing a “0” badge for /tasks
Currently badge is always a string; '0' is truthy and will render a chip. Return undefined when count is zero.
{
path: '/tasks',
names: ['tasks'],
title: t('app.my-tasks'),
icon: 'i-lucide-layout-dashboard',
- badge: taskStore.myTodayTasks.length.toString(),
+ badge: taskStore.myTodayTasks.length > 0
+ ? String(taskStore.myTodayTasks.length)
+ : undefined,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| path: '/tasks', | |
| name: 'tasks', | |
| names: ['tasks'], | |
| title: t('app.my-tasks'), | |
| icon: 'i-lucide-layout-dashboard', | |
| badge: taskStore.myTodayTasks.length.toString(), | |
| }, | |
| path: '/tasks', | |
| names: ['tasks'], | |
| title: t('app.my-tasks'), | |
| icon: 'i-lucide-layout-dashboard', | |
| badge: taskStore.myTodayTasks.length > 0 | |
| ? String(taskStore.myTodayTasks.length) | |
| : undefined, | |
| }, |
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/composables/useNavigation.ts around lines 23 to 28,
the badge is always set to a string which renders a "0" chip; change the badge
to return undefined when the task count is zero. Replace the current badge
expression with a conditional that checks taskStore.myTodayTasks.length === 0
and returns undefined in that case, otherwise returns the numeric count as a
string (or number) so no "0" badge is shown.
| const userStore = useUserStore() | ||
|
|
||
| const myLists = computed(() => | ||
| lists.value.filter( | ||
| (taskList) => taskList.chat?.members.some((member) => member.userId === userStore.id), | ||
| ).filter((taskList) => isTodayOnly.value ? taskList.tasks.filter((task) => !task.completedAt && task.date && isToday(parseDate(task.date), getLocalTimeZone())).length : true), | ||
| ) | ||
| const myTodayTasks = computed(() => myLists.value.flatMap((taskList) => taskList.tasks.filter((task) => !task.completedAt && task.date && isToday(parseDate(task.date), getLocalTimeZone())))) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Predicate returns number; fix boolean logic and avoid repeated TZ lookups
The second filter relies on a numeric length for truthiness, which is a TS type mismatch. Also, you call getLocalTimeZone repeatedly inside tight loops.
const userStore = useUserStore()
- const myLists = computed(() =>
- lists.value.filter(
- (taskList) => taskList.chat?.members.some((member) => member.userId === userStore.id),
- ).filter((taskList) => isTodayOnly.value ? taskList.tasks.filter((task) => !task.completedAt && task.date && isToday(parseDate(task.date), getLocalTimeZone())).length : true),
- )
- const myTodayTasks = computed(() => myLists.value.flatMap((taskList) => taskList.tasks.filter((task) => !task.completedAt && task.date && isToday(parseDate(task.date), getLocalTimeZone()))))
+ const tz = getLocalTimeZone()
+ const isTaskForToday = (task: Task) =>
+ !task.completedAt && task.date && isToday(parseDate(task.date), tz)
+
+ const myLists = computed(() =>
+ lists.value
+ .filter((taskList) =>
+ taskList.chat?.members.some((member) => member.userId === userStore.id),
+ )
+ .filter((taskList) => !isTodayOnly.value || taskList.tasks.some(isTaskForToday)),
+ )
+ const myTodayTasks = computed(() =>
+ myLists.value.flatMap((taskList) => taskList.tasks.filter(isTaskForToday)),
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const userStore = useUserStore() | |
| const myLists = computed(() => | |
| lists.value.filter( | |
| (taskList) => taskList.chat?.members.some((member) => member.userId === userStore.id), | |
| ).filter((taskList) => isTodayOnly.value ? taskList.tasks.filter((task) => !task.completedAt && task.date && isToday(parseDate(task.date), getLocalTimeZone())).length : true), | |
| ) | |
| const myTodayTasks = computed(() => myLists.value.flatMap((taskList) => taskList.tasks.filter((task) => !task.completedAt && task.date && isToday(parseDate(task.date), getLocalTimeZone())))) | |
| const userStore = useUserStore() | |
| const tz = getLocalTimeZone() | |
| const isTaskForToday = (task: Task) => | |
| !task.completedAt && task.date && isToday(parseDate(task.date), tz) | |
| const myLists = computed(() => | |
| lists.value | |
| .filter((taskList) => | |
| taskList.chat?.members.some((member) => member.userId === userStore.id), | |
| ) | |
| .filter((taskList) => !isTodayOnly.value || taskList.tasks.some(isTaskForToday)), | |
| ) | |
| const myTodayTasks = computed(() => | |
| myLists.value.flatMap((taskList) => taskList.tasks.filter(isTaskForToday)), | |
| ) |
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/stores/task.ts around lines 19 to 27, the second
.filter uses a numeric .length as a boolean (causing a TS type mismatch) and
calls getLocalTimeZone() repeatedly inside loops; change the predicate to
explicitly return a boolean (e.g., use .some(...) or .filter(...).length > 0)
and compute getLocalTimeZone() once outside the computed to reuse the value, and
also avoid redundant parseDate calls by evaluating task date/isToday once per
task (e.g., create a small helper or reuse a local variable inside the
callbacks) so the filters return proper booleans and eliminate repeated TZ
lookups.
1 participant
Summary by CodeRabbit
New Features
Improvements
Style