chore: epic comments rework#148
Conversation
WalkthroughUI config switches ring-muted to ring-default, adds a Drawer slots config, and adjusts button/modal classes. CSS gains scrollbar-hiding, header fonts, and a tg-text-inverted utility. Navigation reduces routes and restructures to sticky 3-column layout. Drawer control in epic page moves to v-model. Init flow adds closing confirmation, swipe handling, and gyroscope event. Comment schema text limit increases. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Epic Page
participant Drawer
participant Form as FormCreateEpicComment
User->>Page: Tap "Add Comment"
Page->>Drawer: set isDrawerOpened = true (v-model:open)
activate Drawer
Drawer->>Form: Render in #body
User->>Form: Submit comment
Form-->>Page: emitted 'submitted' / 'success'
Page->>Drawer: set isDrawerOpened = false
deactivate Drawer
Note over Page,Drawer: Drawer visibility is state-driven
sequenceDiagram
autonumber
participant App
participant SDK as Telegram SDK
participant Viewport
App->>SDK: closingBehavior.enableConfirmation.ifAvailable()
App->>SDK: mountSwipeBehavior.ifAvailable()
App->>SDK: disableVerticalSwipes.ifAvailable()
App->>SDK: emit web_app_start_gyroscope{ refresh_rate:80 }
App->>SDK: mountMiniAppSync.ifAvailable()
App->>SDK: bindThemeParamsCssVars.ifAvailable()
App->>Viewport: lockOrientation / requestFullscreen / mount
Note over App,SDK: ifAvailable() guards all optional features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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
🧹 Nitpick comments (8)
apps/atrium-telegram/app/app.vue (1)
71-71: Interval typing + avoid overlapping refreshes
- Use ReturnType in browser code.
- Guard against overlapping Promise.all runs (slow network can stack intervals).
Apply:
-// Auto Update -let interval: NodeJS.Timeout +// Auto Update +let interval: ReturnType<typeof setInterval> onMounted(async () => { - await Promise.all([ - user.updateOnline(), - user.update(), - task.update(), - epic.update(), - ]) - - interval = setInterval(async () => { - await Promise.all([ - user.updateOnline(), - user.update(), - task.update(), - epic.update(), - ]) - }, 30000) + let inFlight = false + const tick = async () => { + if (inFlight) return + inFlight = true + try { + await Promise.all([ + user.updateOnline(), + user.update(), + task.update(), + epic.update(), + ]) + } finally { + inFlight = false + } + } + await tick() + interval = setInterval(tick, 30000) }) onUnmounted(() => { clearInterval(interval) })Also applies to: 72-74, 82-90, 92-94
apps/atrium-telegram/shared/services/epic.ts (1)
16-16: Comment length 3000: align backend, UX, and messages
- Ensure API and DB column sizes accept 3k (including encoding), and server-side validation matches.
- Update i18n error text if it mentions 1500.
- Consider a live char counter and soft-truncation in lists to avoid layout shifts.
apps/atrium-telegram/nuxt.config.ts (1)
38-39: Fonts: avoid double-loading and verify mapping to font-headers
- If using the top-level fonts module, consider disabling UI’s built-in fonts to prevent duplicate loads.
- Verify that the Tailwind/ CSS variable for font-headers maps to the loaded families; otherwise headers may fall back unexpectedly.
- Prefer font loading with “swap” behavior.
Apply if appropriate:
ui: { colorMode: true, - fonts: true, + fonts: false, },Also applies to: 43-64
apps/atrium-telegram/app/assets/css/styles.css (2)
113-115: Scope scrollbar hiding to opt-in containersGlobal scrollbar hiding on html/body can hurt discoverability and accessibility. Keep the new .hide-scroll utility and drop global hiding.
Apply:
html, body { background: var(--tg-theme-secondary-bg-color, transparent) !important; - -ms-overflow-style: none; /* IE and Edge */ - scrollbar-width: none; /* Firefox */ } -body::-webkit-scrollbar { - display: none; - width: 0; -} - .hide-scroll { -ms-overflow-style: none; /* IE and Edge */ scrollbar-width: none; /* Firefox */ } .hide-scroll::-webkit-scrollbar { display: none; width: 0; }Also applies to: 117-121, 122-131
132-134: Add a safe fallback for header font variableIf --font-headers isn’t set yet, fall back to system sans.
Apply:
-h1, h2, h3, h4, h5, h6 { - font-family: var(--font-headers); -} +h1, h2, h3, h4, h5, h6 { + font-family: var(--font-headers, ui-sans-serif, system-ui, -apple-system, Segoe UI, Roboto); +}apps/atrium-telegram/app/components/EpicComment.vue (1)
67-74: Clipboard UX + remove dead onSelect on disabled item
- Clipboard write should be awaited with error handling and user feedback.
- Disabled beacon item still has onSelect; drop it to avoid confusion.
Apply:
- { - label: 'Скопировать сообщение', - icon: 'i-lucide-copy', - color: 'neutral', - disabled: false, - onSelect: () => navigator.clipboard.writeText(comment.value?.text ?? ''), - condition: true, - }, + { + label: 'Скопировать сообщение', + icon: 'i-lucide-copy', + color: 'neutral', + disabled: false, + onSelect: async () => { + try { + await navigator.clipboard.writeText(comment.value?.text ?? '') + useToast().add({ title: 'Скопировано' }) + } catch { + useToast().add({ title: 'Не удалось скопировать', color: 'red' }) + } + }, + condition: true, + }, @@ - { - label: 'Маякнуть (будет позже)', - icon: 'i-lucide-users-round', - color: 'neutral', - disabled: true, - onSelect: () => modalCreateEpicCommentBeacon.open({ commentId }), - condition: true, - }, + { + label: 'Маякнуть (будет позже)', + icon: 'i-lucide-users-round', + color: 'neutral', + disabled: true, + condition: true, + },Also applies to: 75-79
apps/atrium-telegram/app/utils/init.ts (1)
80-82: Nice use of .ifAvailable; add gyroscope stop + confirm UX check
- Good: migrating to .ifAvailable for mounts.
- Gyroscope: start without a corresponding stop can drain battery. Stop on tab hide; resume on show.
- Confirm close: verify you still provide an explicit “close” path to avoid trapping users.
Apply:
// Gyroscope - postEvent('web_app_start_gyroscope', { - refresh_rate: 80, - }) + const startGyro = () => postEvent('web_app_start_gyroscope', { refresh_rate: 80 }) + const stopGyro = () => postEvent('web_app_stop_gyroscope') + startGyro() + document.addEventListener('visibilitychange', () => { + if (document.hidden) stopGyro() + else startGyro() + })Also applies to: 84-84, 86-89, 95-99
apps/atrium-telegram/app/pages/epic/[epicId]/index.vue (1)
41-55: Confirm Drawer trigger behavior; add explicit open as fallbackIf UDrawer’s default slot isn’t treated as a trigger with v-model, clicking CreateCard won’t open it. Add an explicit click to set isDrawerOpened.
Apply if needed:
- <CreateCard + <CreateCard v-if="epic?.id" :label="$t('app.create.epic-comment.button')" icon="i-lucide-message-circle" + @click="isDrawerOpened = true" />Also applies to: 68-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/atrium-telegram/app/app.config.ts(2 hunks)apps/atrium-telegram/app/app.vue(1 hunks)apps/atrium-telegram/app/assets/css/styles.css(2 hunks)apps/atrium-telegram/app/components/EpicComment.vue(2 hunks)apps/atrium-telegram/app/components/Navigation.vue(2 hunks)apps/atrium-telegram/app/components/form/CreateEpicComment.vue(0 hunks)apps/atrium-telegram/app/pages/epic/[epicId]/index.vue(2 hunks)apps/atrium-telegram/app/pages/secret2.vue(0 hunks)apps/atrium-telegram/app/utils/init.ts(2 hunks)apps/atrium-telegram/nuxt.config.ts(1 hunks)apps/atrium-telegram/shared/services/epic.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/atrium-telegram/app/components/form/CreateEpicComment.vue
- apps/atrium-telegram/app/pages/secret2.vue
⏰ 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 (5)
apps/atrium-telegram/app/assets/css/styles.css (1)
70-72: Utility LGTMtg-text-inverted is consistent with your token model.
apps/atrium-telegram/app/components/EpicComment.vue (1)
10-12: Width tweak LGTMDropdown width increase to w-56 improves long-label fit.
apps/atrium-telegram/app/app.config.ts (1)
5-6: Token updates LGTM; verify ring-default + drawer slots integration
- ring-muted → ring-default is consistent; ensure ring-default is defined in your Tailwind tokens.
- drawer slots (header/body/content) align with usage; hide-scroll usage matches CSS utility.
Also applies to: 10-12, 15-16, 20-21, 25-26, 30-31, 54-54, 59-65
apps/atrium-telegram/app/components/Navigation.vue (2)
4-4: Grid change aligns with route count—LGTM.
grid-cols-3matches the three items inmainRoutes.
29-30: Closing tag order matches the new wrapper hierarchy—LGTM.No issues with structure after the wrapper/nav swap.
| <div class="z-50 touch-pan-x sticky inset-0 h-24"> | ||
| <nav class="w-full h-24 tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up"> |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Sticky bottom bar likely mispositioned: replace inset-0 with bottom-0 inset-x-0 and add safe-area padding.
position: sticky needs one offset side. inset-0 sets both top:0 and bottom:0, which tends to pin to the top and can behave inconsistently. For a bottom nav, prefer bottom-0 inset-x-0 (or revert to fixed bottom-0 inset-x-0 if you want it always visible). Also add iOS safe-area padding and an aria label.
Test on iOS Safari and Telegram WebApp to ensure it sticks to the bottom and doesn’t overlap the home indicator.
- <div class="z-50 touch-pan-x sticky inset-0 h-24">
- <nav class="w-full h-24 tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up">
+ <div class="z-50 touch-pan-x sticky bottom-0 inset-x-0 h-24">
+ <nav aria-label="Primary navigation" class="w-full h-24 pb-[env(safe-area-inset-bottom)] tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up">If you intended a top bar instead, use sticky top-0 inset-x-0 on the wrapper.
Fix sticky bottom nav: use bottom-0 inset-x-0, add safe-area padding and nav aria-label
File: apps/atrium-telegram/app/components/Navigation.vue (lines 2-3). position:sticky needs a directional offset; inset-0 sets top+bottom and can pin/misposition the bar — replace as below, add safe-area padding and aria-label, then test on iOS Safari and Telegram WebApp.
- <div class="z-50 touch-pan-x sticky inset-0 h-24">
- <nav class="w-full h-24 tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up">
+ <div class="z-50 touch-pan-x sticky bottom-0 inset-x-0 h-24">
+ <nav aria-label="Primary navigation" class="w-full h-24 pb-[env(safe-area-inset-bottom)] tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up">📝 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.
| <div class="z-50 touch-pan-x sticky inset-0 h-24"> | |
| <nav class="w-full h-24 tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up"> | |
| <div class="z-50 touch-pan-x sticky bottom-0 inset-x-0 h-24"> | |
| <nav aria-label="Primary navigation" class="w-full h-24 pb-[env(safe-area-inset-bottom)] tg-bg-bottom-bar border-t border-default rounded-t-lg motion-preset-slide-up"> |
🤖 Prompt for AI Agents
In apps/atrium-telegram/app/components/Navigation.vue around lines 2-3, change
the root container classes so the sticky bar is anchored to the bottom (replace
sticky inset-0 with sticky bottom-0 inset-x-0 h-24), add safe-area bottom
padding to respect iOS notch (e.g. apply padding-bottom using
env(safe-area-inset-bottom) or a Tailwind utility like
pb-[env(safe-area-inset-bottom)] on the container/nav), and add an accessible
label to the <nav> element (aria-label="Bottom navigation"); then test the bar
on iOS Safari and Telegram WebApp to confirm it stays pinned and is not
overlapped by the home indicator.
1 participant
Summary by CodeRabbit
New Features
UI/Style
Changes
Chores