Release v0.3.5 — Phase 48: Mobile UX Pass - #900
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gressions RG-755: Audit Mobile UX Regressions
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t-browser-test-helpers RG-756: Add Mobile Viewport Browser Test Helpers
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…zontal-overflow-smoke-test RG-757: Add Mobile No Horizontal Overflow Smoke Test
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ader-and-navigation RG-758: Optimize Mobile Header And Navigation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed-spacing RG-759: Optimize Mobile Feed Spacing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st-card-layout RG-760: Optimize Mobile Post Card Layout
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st-show-layout RG-761: Optimize Mobile Post Show Layout
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…wer-to-full-screen-sheet RG-762: Convert Mobile Drawer To Full-Screen Sheet
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ting-options-layout RG-763: Optimize Mobile Rating Options Layout
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…load-form RG-764: Optimize Mobile Upload Form
Add overflow-hidden/min-w-0 to comments section; comment item grid already uses minmax(0,1fr) and break-words for safe mobile layout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mments-layout RG-765: Optimize mobile comments layout
Truncate long display names to prevent overflow; add tests for profile header structure and mobile-safe identity container. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ofile-layout RG-766: Optimize mobile profile layout
Constrain dropdown to max-w-[calc(100vw-2rem)] to prevent viewport overflow on narrow screens; add break-words to notification content. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tifications-dropdown RG-767: Optimize mobile notifications dropdown
Bump locale switcher trigger from h-9 to h-10 (40px) to meet minimum tap target requirement on mobile screens. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nguage-and-theme-switchers RG-768: Optimize mobile language and theme switchers
Add data-testid to register form; login already uses flex-wrap for mobile-safe action row; guest layout uses px-4 and max-w-md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…th-pages RG-769: Optimize mobile auth pages
Add data-screenshot attributes to key views: feed-page, profile-header, and auth-page for visual regression test tooling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…screenshot-targets RG-770: Add mobile visual screenshot targets
Add phase-48-visual-baselines.md documenting accepted visual state, screenshot targets, viewports, and acceptance criteria for 375px. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…al-baselines RG-771: Update mobile visual baselines
Add Mobile QA section with 5 checklist items covering 375px overflow, tap targets, text wrapping, drawer behavior, and header switchers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…klist-to-pr-template RG-772: Add mobile QA checklist to PR template
Add mobile-ux-guidelines.md covering overflow prevention, tap targets, text wrapping, responsive display, drawer behavior, and breakpoints. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mentation RG-773: Add mobile UX documentation
Add complete review checklist with all 20 tasks marked done, final acceptance criteria, and phase completion date. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-ux-review-checklist RG-774: Add phase 48 mobile UX review checklist
overflow-hidden on the feed-page root div broke position:sticky on the detail panel — any overflow value other than visible on an ancestor kills sticky in descendants. The detail panel appeared at the top of the document instead of staying fixed in the viewport while the user was scrolled down. On mobile (< lg) the detail panel is in document flow below the feed, so scrollToSelectedPost now scrolls to the panel instead of the card. A double-rAF ensures positions are read after the two-column layout has fully settled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix post detail panel invisible when clicking mid-feed post
Previous vendor view used Tailwind utility classes (bg-rg-card etc.) built from PHP variables — Tailwind's scanner cannot detect classes stored in runtime variables, so they were absent from the CSS bundle, making the pagination ignore the active theme. New approach: define rg-page-btn / rg-page-btn-active / rg-page-btn-off as @layer components rules in app.css using raw CSS variables (--rg-card, --rg-text-2, --rg-accent-soft, etc.). These are always emitted regardless of content scanning, and inherit the correct values from the html[data-theme] overrides at runtime. Also reduced button size from h-8 (32px) to h-[26px] and icons from size-3.5 to size-3 for a more compact pagination bar. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix pagination: compact size and reliable theme responsiveness
Theme-responsive colors: moved styles into @once <style> block inside the blade view using raw CSS variables (var(--rg-card), var(--rg-text-2), etc.). This bypasses Tailwind's content scanner and @layer cascade entirely — styles are inlined in the HTML and resolved by the browser at runtime against the active html[data-theme] values. Page window logic: always shows pages 1-2-3, current page if it falls outside that group, and last two pages, with ellipsis between non-consecutive groups. Single-page gaps are filled to avoid a lone ellipsis (e.g. "3 … 5" becomes "3 4 5"). When total pages ≤ 7 all pages are shown without ellipsis. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix pagination colors and page window
Livewire's WithPagination uses the livewire::tailwind view (resources/views/vendor/livewire/tailwind.blade.php), not the standard Laravel pagination::tailwind view. Previous overrides were placed at the wrong path and had no effect — the default Livewire view with hardcoded bg-white was rendered instead. New vendor view: - Correct path: resources/views/vendor/livewire/tailwind.blade.php - wire:click="gotoPage/previousPage/nextPage" preserved for Livewire - @once <style> block with CSS variables guarantees theme-responsive colors without any Tailwind build dependency - Custom page window: always shows 1-2-3, current page if in the middle, last two pages; fills single-page gaps to avoid lone ellipsis - Disables scrollTo auto-scroll (handled by feed-page Alpine logic) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix pagination: correct Livewire vendor view path
|
Warning Review limit reached
More reviews will be available in 37 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughКомплексная фаза 48 мобильного UX с внедрением пагинации в ленту постов (Livewire WithPagination + два Blade-шаблона пагинации), обновлением CSS-стилей (кнопки пагинации, переменные теней), исправлением мобильных вёрсток (min-w-0 для overflow, break-words/truncate для текста, tap targets 40px), добавлением data-testid/data-screenshot для тестирования и созданием инфраструктуры мобильных тестов (MobileViewports, 20+ Feature-тестов, браузерные smoke-тесты на 375px) с полной документацией (guidelines, audit, checklists, baselines). ChangesPhase 48 Mobile UX Pass
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
14 issues found across 42 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="resources/css/app.css">
<violation number="1" location="resources/css/app.css:35">
P2: Dead/duplicate code: `.rg-page-btn*` classes in `app.css` are never referenced anywhere and duplicate the `rg-pgn-btn*` styles already defined inline in `resources/views/vendor/livewire/tailwind.blade.php`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| --tw-ring-offset-shadow: 0 0 #0000; | ||
| } | ||
|
|
||
| .rg-page-btn { |
There was a problem hiding this comment.
P2: Dead/duplicate code: .rg-page-btn* classes in app.css are never referenced anywhere and duplicate the rg-pgn-btn* styles already defined inline in resources/views/vendor/livewire/tailwind.blade.php.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At resources/css/app.css, line 35:
<comment>Dead/duplicate code: `.rg-page-btn*` classes in `app.css` are never referenced anywhere and duplicate the `rg-pgn-btn*` styles already defined inline in `resources/views/vendor/livewire/tailwind.blade.php`.</comment>
<file context>
@@ -31,4 +31,44 @@
--tw-ring-offset-shadow: 0 0 #0000;
}
+
+ .rg-page-btn {
+ display: inline-flex;
+ align-items: center;
</file context>
Post-release fixes (previously uncommitted):
- app.blade.php: remove overflow-hidden from header flex container — clips
all absolutely-positioned dropdown menus (P0)
- theme.css: add --rg-shadow-dropdown token and light-theme shadow overrides
- PostFeed: switch ->get() to ->paginate(12) with WithPagination trait;
pass $paginator to view for pagination links display
Code violations:
- comments-section: remove overflow-hidden — clips the sort dropdown
(.absolute.right-0.z-20.mt-2); min-w-0 alone is sufficient (P2)
- rating-options: replace hardcoded data-testid="rating-options" with
data-testid="{{ $testIdPrefix }}-list" so multiple groups on one page
produce unique IDs (P2)
- app.css + livewire pagination view: increase tap target from 26px to
40px to meet project ≥40px mobile requirement (P1)
Doc violations:
- phase-48-visual-baselines.md: fix route /profile/:user → /u/{username} (P2)
- mobile-ux-guidelines.md: replace overflow-hidden recommendation with
min-w-0 + warning about sticky/dropdown breakage (P1);
fix breakpoints table — 375px is a test viewport, not a mobile: prefix (P2)
Test violations:
- MobileSwitchersTest: scope h-10 check to locale-switcher-trigger context
instead of full page body (P2 false positive)
- MobileFeedSpacingTest: rename tests to accurately describe HTML structure
checks, not mobile-specific behavior (P2)
- MobileRatingVotingTest: use post->id in container testid assertion;
replace substr_count with preg_match_all to match only individual option
buttons, not the container (P2)
- MobileUploadFormTest: rename misleading test descriptions (P3)
- Phase48ReviewChecklistTest: rename to "references first and last task IDs" (P3)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix Phase 48 review violations and commit post-release fixes
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/mobile/mobile-ux-guidelines.md`:
- Around line 70-73: The fenced code block in
docs/mobile/mobile-ux-guidelines.md lacks a language identifier; update the
triple-backtick fence that surrounds the lines starting with "mobile:
inset-x-0..." and "desktop: md:inset-y-0..." to include a language tag (e.g.,
css or text) so the block becomes ```css (or ```text) to ensure proper Markdown
rendering and syntax highlighting.
In `@resources/views/components/voting/rating-options.blade.php`:
- Line 24: Update the Tailwind "important" modifier usage in the $sizeClass
assignment: replace prefix-style modifiers with suffix-style ones per Tailwind
v4. In the expression that sets $sizeClass (the ternary using $variant ===
'compact' and the else branch), change "!h-7 !min-w-9 !px-2 !text-xs" to "h-7!
min-w-9! px-2! text-xs!" and change "!min-h-[40px]" to "min-h-[40px]!" so all
utilities use the trailing "!" form.
In `@resources/views/layouts/app.blade.php`:
- Line 19: Уберите пустой элемент <span class="sr-only"> с атрибутом
data-testid="mobile-header" и переместите data-testid="mobile-header" на
реальный структурный элемент заголовка (например, на элемент header,
используемый в шаблоне — смотрите текущий header вокруг span), чтобы не
оставлять пустой скрытый элемент и сохранить тестовый идентификатор на
функциональном семантическом элементе; удалите сам пустой span и убедитесь, что
header остаётся доступным для скринридеров (не добавляйте видимый текст туда).
In `@resources/views/livewire/feed/feed-page.blade.php`:
- Around line 15-37: Add a short inline comment above the nested
requestAnimationFrame calls explaining why two RAF ticks are required (e.g., to
allow the detail panel's layout to fully settle before
measuring/getBoundingClientRect and performing smooth scroll), referencing the
nested requestAnimationFrame block that uses this.$refs.detailScroll/detail and
the subsequent measurements of detail and feed/card
(querySelector('[data-post-id=' + postId + ']')) so future readers know this is
intentional and not accidental.
In `@resources/views/livewire/posts/post-show.blade.php`:
- Line 29: The empty <span class="sr-only" data-testid="post-show-page"> is
semantically incorrect; move the data-testid attribute from that empty <span> to
an actual functional container such as the <div data-testid="post-show">
(element at line ~16) or the <main> element (around line ~30) so the test id
attaches to a meaningful DOM element and remove the empty sr-only span; update
or remove the <span data-testid="post-show-page"> accordingly to avoid leaving
an empty, screen-reader-only element.
In `@resources/views/vendor/pagination/tailwind.blade.php`:
- Around line 7-8: Pagination buttons in
resources/views/vendor/pagination/tailwind.blade.php use height: 26px and
min-width: 26px which breaks the 40px mobile tap target requirement; change
those values to 40px to match the Livewire template and also update the
.rg-pgn-ellipsis selector (used for the ellipsis/gap control) so its height and
min-width match 40px for consistent visual alignment across both pagination
variants.
In `@tests/Browser/MobileOverflowSmokeTest.php`:
- Around line 9-59: These mobile overflow smoke tests call
visit(...)->resize(...)->script(...) immediately and can miss async Livewire
hydration adding overflow; update each test case that measures overflow (the it
blocks using visit(...)->resize(...)->script(...) with
MobileViewports::SMALL_MOBILE) to wait for Livewire (e.g., insert a
->waitForLivewire() before ->script()) or add a short ->pause(500) to ensure
async content finished before measuring scrollWidth - window.innerWidth.
In `@tests/Feature/Docs/Phase48MobileViewportHelperTest.php`:
- Around line 3-13: The test is inspecting raw file contents which is brittle;
update the test to import or reference the MobileViewports class
(MobileViewports) and assert its public API returns the expected viewport sizes
(e.g., that the relevant public constants or methods return values containing
375, 390, 768) instead of searching the file text; remove the file_get_contents
and file_exists assertions and replace them with assertions against
MobileViewports' public methods or constants (e.g., call the getter or read the
constant names provided by MobileViewports) to verify the actual API behavior.
In `@tests/Feature/Mobile/MobileAuthTest.php`:
- Around line 10-14: The current test in MobileAuthTest.php ("login action row
uses flex-wrap to prevent overflow on mobile") uses assertSee('flex-wrap',
false) which can match substrings like "flex-wrap-reverse"; replace that
assertion with a regex-based check that validates the class attribute contains
the exact token, e.g. assert that the response body matches
/class="[^"]*\bflex-wrap\b[^"]*"/ (or equivalent TestResponse regex helper) to
ensure the class token is present, or alternatively convert the test into a
browser/edge-rendering test that verifies there is no horizontal overflow on a
narrow viewport.
In `@tests/Feature/Mobile/MobileCommentsTest.php`:
- Around line 26-38: The test "long comment text uses break-words to prevent
overflow" in MobileCommentsTest.php is brittle because it asserts a specific
Tailwind class; change it to verify behavior instead: convert this unit-level
Livewire html() test into a browser-style test (e.g., Laravel Dusk or Pest +
Panther) that mounts the CommentsSection component (or the page rendering it)
with the long-word Comment and then asserts the comment element does not
overflow horizontally by checking element.scrollWidth <= element.clientWidth or
by using getComputedStyle(element).overflowWrap !== 'normal'; alternatively, if
you must keep a server-side assertion, relax the expectation to accept any
wrapping-related style/class by checking the HTML for either 'break-words' or
'overflow-wrap' occurrences rather than a single Tailwind token.
- Around line 16-24: The test in MobileCommentsTest.php is tied to an exact CSS
class (grid-cols-[32px_minmax(0,1fr)]) on the CommentsSection component; replace
this fragile implementation assertion with a browser-level behavior check:
convert the test to a Dusk/Pest browser test that renders the page containing
CommentsSection (the Livewire component referenced as CommentsSection::class),
resize the viewport to a mobile width (e.g., 375px), query the comments
container (add a stable data-testid or CSS selector on the CommentsSection root
if needed), and assert via JS that element.scrollWidth <= element.clientWidth
(or use browser->script to compare scrollWidth and clientWidth). Alternatively,
move this check to your visual-regression suite if preferred.
In `@tests/Feature/Mobile/MobileDrawerTest.php`:
- Around line 5-13: The test it('renders drawer shell with mobile full screen
classes') is asserting specific CSS classes ('w-full', 'bottom-0',
'max-h-[90vh]') which couples it to implementation; change the test to assert
behavior instead — replace the class assertions with a behavior-focused check
such as rendering the drawer in a headless browser with a small viewport (use a
browser test / Dusk/Pest browser test) and assert the drawer fills the viewport
or is anchored to the bottom, or assert a semantic indicator (e.g., presence of
data-testid="drawer-shell" plus an attribute or state like aria-modal/role or a
mobile-mode flag) that denotes mobile layout; update the test name accordingly
and remove the brittle class assertions in MobileDrawerTest.php inside the
it(...) block so future style refactors won't break the test.
In `@tests/Feature/Mobile/MobileFeedSpacingTest.php`:
- Around line 5-23: The test file MobileFeedSpacingTest contains tests that
assert structural data-testid attributes (the tests with descriptions 'feed page
includes expected structural testid markers', 'feed page includes rating filters
container', and 'feed page includes feed layout container') so rename the file
(and any matching class/test suite name if present) from MobileFeedSpacingTest
to MobileFeedStructureTest to reflect its purpose, or alternatively change the
test descriptions to reference spacing if you intend to keep the Spacing name;
update references/imports accordingly.
In `@tests/Feature/Mobile/MobilePostCardTest.php`:
- Around line 28-35: The test name and assertion are misleading: update the test
in MobilePostCardTest (the it(...) block that currently reads "renders post card
with overflow-hidden container") to explicitly verify the mobile post card does
not contain the problematic CSS; either rename the test to reflect the
expectation and replace ->assertSee('overflow-hidden', false) with
->assertDontSee('overflow-hidden') or, better, add/target a specific marker
(e.g. data-testid on the post card component) and assert the response for that
marker does not include "overflow-hidden" so you only check the post card
container (use the Post::factory()->published()->create(),
$this->get(route('feed')) and the response assertions around the specific
container).
In `@tests/Feature/Mobile/MobileSwitchersTest.php`:
- Around line 13-22: The current test uses strpos/substr ($triggerPos, $snippet)
to search for 'h-10' near 'locale-switcher-trigger', which is fragile and can
yield false positives; replace the substring approach by parsing the returned
HTML ($html from $this->get(route('feed'))) with a DOM parser/Crawler (e.g.,
Symfony\Component\DomCrawler\Crawler), locate the element by its data-testid or
selector for locale-switcher-trigger (e.g.,
'[data-testid="locale-switcher-trigger"]' or the element with class/ID used in
the markup), then assert directly that that element's class attribute contains
'h-10' (or that the element matches the selector
'[data-testid="locale-switcher-trigger"].h-10'); update the test to remove
$triggerPos/$snippet and use the Crawler-based lookup and assertion instead.
In `@tests/Feature/Mobile/MobileUploadFormTest.php`:
- Around line 15-24: Тест в MobileUploadFormTest.php ("upload form contains
vertical field spacing") жёстко зависит от реализации (assert
expect($html)->toContain('space-y-4')); уберите или замените эту проверку и
вместо неё либо (а) переведите проверку в браузерный тест (Laravel Dusk) который
рендерит компонент UploadPostForm и использует $browser->script/getComputedStyle
или измеряет расстояние между соседними полями, затем ассертом проверяет
реальный vertical spacing (например gap/margin в пикселях или > 0), либо (б)
замените на семантическую проверку структуры (наличие контейнера upload-form и
ожидаемых полей) если визуальная проверка не требуется; в коде ищите
MobileUploadFormTest::it('upload form contains vertical field spacing'),
Livewire::actingAs(...)->test(UploadPostForm::class)->html() и удалите/замените
строку expect($html)->toContain('space-y-4') согласно одному из подходов.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7382b449-3fe2-4126-8ba6-bbb667ad784f
📒 Files selected for processing (45)
.github/pull_request_template.mdapp/Livewire/Feed/PostFeed.phpdocs/mobile/mobile-ux-guidelines.mddocs/mobile/phase-48-mobile-ux-audit.mddocs/mobile/phase-48-review-checklist.mddocs/mobile/phase-48-visual-baselines.mdresources/css/app.cssresources/css/theme.cssresources/views/auth/register.blade.phpresources/views/components/locale-switcher.blade.phpresources/views/components/ui/drawer.blade.phpresources/views/components/voting/rating-options.blade.phpresources/views/layouts/app.blade.phpresources/views/layouts/guest.blade.phpresources/views/livewire/comments/comments-section.blade.phpresources/views/livewire/feed/feed-page.blade.phpresources/views/livewire/feed/post-feed.blade.phpresources/views/livewire/feed/upload-post-form.blade.phpresources/views/livewire/notifications/notification-bell.blade.phpresources/views/livewire/posts/post-show.blade.phpresources/views/livewire/profile/profile-page.blade.phpresources/views/vendor/livewire/tailwind.blade.phpresources/views/vendor/pagination/tailwind.blade.phptests/Browser/MobileOverflowSmokeTest.phptests/Browser/Support/MobileViewports.phptests/Feature/Docs/Phase48MobileBaselinesTest.phptests/Feature/Docs/Phase48MobileOverflowSmokeTest.phptests/Feature/Docs/Phase48MobileUxAuditTest.phptests/Feature/Docs/Phase48MobileUxDocTest.phptests/Feature/Docs/Phase48MobileViewportHelperTest.phptests/Feature/Docs/Phase48PrTemplateTest.phptests/Feature/Docs/Phase48ReviewChecklistTest.phptests/Feature/Mobile/MobileAuthTest.phptests/Feature/Mobile/MobileCommentsTest.phptests/Feature/Mobile/MobileDrawerTest.phptests/Feature/Mobile/MobileFeedSpacingTest.phptests/Feature/Mobile/MobileHeaderTest.phptests/Feature/Mobile/MobileNotificationsTest.phptests/Feature/Mobile/MobilePostCardTest.phptests/Feature/Mobile/MobilePostShowTest.phptests/Feature/Mobile/MobileProfileTest.phptests/Feature/Mobile/MobileRatingVotingTest.phptests/Feature/Mobile/MobileScreenshotTargetsTest.phptests/Feature/Mobile/MobileSwitchersTest.phptests/Feature/Mobile/MobileUploadFormTest.php
| <div class="mx-auto flex h-[60px] w-full max-w-[1440px] items-center gap-4 px-5 md:grid md:grid-cols-[1fr_minmax(0,480px)_auto]"> | ||
| <a href="{{ url('/') }}" class="shrink-0 self-center rounded-rgControl px-2 py-1 text-[22px] font-extrabold tracking-normal text-rg-text transition-colors hover:text-rg-accent focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-rg-accent focus-visible:ring-offset-2 focus-visible:ring-offset-rg-bg" data-testid="site-brand"> | ||
| {{ $projectSettings->siteName() }} | ||
| <span data-testid="mobile-header" class="sr-only"></span> |
There was a problem hiding this comment.
Пустой sr-only элемент семантически некорректен.
Класс sr-only предназначен для скрытия визуального контента, оставляя его доступным для программ чтения с экрана. Пустой <span class="sr-only"> без текстового содержимого не несёт семантической нагрузки и может вызвать путаницу в инструментах доступности.
Рекомендация: переместите data-testid="mobile-header" на функциональный элемент заголовка (например, на <header> на строке 18) вместо создания отдельного пустого маркера.
♻️ Предлагаемое исправление
- <header class="sticky top-0 z-40 border-b border-rg-border bg-rg-topbar" data-testid="app-header">
- <span data-testid="mobile-header" class="sr-only"></span>
+ <header class="sticky top-0 z-40 border-b border-rg-border bg-rg-topbar" data-testid="app-header" data-mobile-testid="mobile-header">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/views/layouts/app.blade.php` at line 19, Уберите пустой элемент
<span class="sr-only"> с атрибутом data-testid="mobile-header" и переместите
data-testid="mobile-header" на реальный структурный элемент заголовка (например,
на элемент header, используемый в шаблоне — смотрите текущий header вокруг
span), чтобы не оставлять пустой скрытый элемент и сохранить тестовый
идентификатор на функциональном семантическом элементе; удалите сам пустой span
и убедитесь, что header остаётся доступным для скринридеров (не добавляйте
видимый текст туда).
Comment on lines
+5
to
+13
| it('renders drawer shell with mobile full screen classes', function () { | ||
| $html = Blade::render('<x-ui.drawer title="Post details">Content</x-ui.drawer>'); | ||
|
|
||
| expect($html) | ||
| ->toContain('data-testid="drawer-shell"') | ||
| ->toContain('w-full') | ||
| ->toContain('bottom-0') | ||
| ->toContain('max-h-[90vh]'); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Тест проверяет конкретные CSS-классы вместо поведения.
Утверждения toContain('w-full'), toContain('bottom-0'), toContain('max-h-[90vh]') привязывают тест к текущей реализации стилей. Если drawer будет переработан с сохранением визуального результата (полный экран на мобильном), тест всё равно упадёт. Для валидации мобильного поведения эффективнее browser-тесты с реальным viewport или visual regression.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Feature/Mobile/MobileDrawerTest.php` around lines 5 - 13, The test
it('renders drawer shell with mobile full screen classes') is asserting specific
CSS classes ('w-full', 'bottom-0', 'max-h-[90vh]') which couples it to
implementation; change the test to assert behavior instead — replace the class
assertions with a behavior-focused check such as rendering the drawer in a
headless browser with a small viewport (use a browser test / Dusk/Pest browser
test) and assert the drawer fills the viewport or is anchored to the bottom, or
assert a semantic indicator (e.g., presence of data-testid="drawer-shell" plus
an attribute or state like aria-modal/role or a mobile-mode flag) that denotes
mobile layout; update the test name accordingly and remove the brittle class
assertions in MobileDrawerTest.php inside the it(...) block so future style
refactors won't break the test.
Comment on lines
+5
to
+23
| it('feed page includes expected structural testid markers', function () { | ||
| Post::factory()->published()->create(['title' => 'Feed Spacing Test']); | ||
|
|
||
| $this->get(route('feed')) | ||
| ->assertOk() | ||
| ->assertSee('data-testid="feed-page"', false); | ||
| }); | ||
|
|
||
| it('feed page includes rating filters container', function () { | ||
| $this->get(route('feed')) | ||
| ->assertOk() | ||
| ->assertSee('data-testid="feed-rating-filters"', false); | ||
| }); | ||
|
|
||
| it('feed page includes feed layout container', function () { | ||
| $this->get(route('feed')) | ||
| ->assertOk() | ||
| ->assertSee('data-testid="feed-layout"', false); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Название файла не соответствует содержанию тестов.
Файл называется MobileFeedSpacingTest, но тесты проверяют наличие структурных data-testid атрибутов, а не spacing (отступы/промежутки). Если планируется добавить проверки spacing позже, оставьте как есть; иначе переименуйте в MobileFeedStructureTest для ясности.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Feature/Mobile/MobileFeedSpacingTest.php` around lines 5 - 23, The test
file MobileFeedSpacingTest contains tests that assert structural data-testid
attributes (the tests with descriptions 'feed page includes expected structural
testid markers', 'feed page includes rating filters container', and 'feed page
includes feed layout container') so rename the file (and any matching class/test
suite name if present) from MobileFeedSpacingTest to MobileFeedStructureTest to
reflect its purpose, or alternatively change the test descriptions to reference
spacing if you intend to keep the Spacing name; update references/imports
accordingly.
Comment on lines
+28
to
+35
| it('renders post card with overflow-hidden container', function () { | ||
| Post::factory()->published()->create(); | ||
|
|
||
| $response = $this->get(route('feed')); | ||
|
|
||
| $response->assertOk() | ||
| ->assertSee('overflow-hidden', false); | ||
| }); |
There was a problem hiding this comment.
Название и логика теста не соответствуют цели мобильного UX.
Тест с названием "renders post card with overflow-hidden container" проверяет, что overflow-hidden присутствует где-то на странице feed через assertSee('overflow-hidden', false). Однако:
- В контексте Phase 48 цель — предотвратить горизонтальный overflow и убрать
overflow-hiddenиз проблемных мест (PR description упоминает "Removed overflow-hidden that broke position:sticky"). - Текущий
assertSeeслишком широкий — он срабатывает, еслиoverflow-hiddenвстречается где угодно в HTML, включая допустимые места (например, в других компонентах). - Неясно, что именно проверяет тест: наличие overflow-hidden (плохо) или его отсутствие (хорошо)?
Рекомендация: либо переименовать тест и уточнить его назначение, либо заменить на assertDontSee('overflow-hidden') для контейнера карточки поста, либо использовать более точный селектор/data-testid для проверки конкретного контейнера.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Feature/Mobile/MobilePostCardTest.php` around lines 28 - 35, The test
name and assertion are misleading: update the test in MobilePostCardTest (the
it(...) block that currently reads "renders post card with overflow-hidden
container") to explicitly verify the mobile post card does not contain the
problematic CSS; either rename the test to reflect the expectation and replace
->assertSee('overflow-hidden', false) with ->assertDontSee('overflow-hidden')
or, better, add/target a specific marker (e.g. data-testid on the post card
component) and assert the response for that marker does not include
"overflow-hidden" so you only check the post card container (use the
Post::factory()->published()->create(), $this->get(route('feed')) and the
response assertions around the specific container).
Comment on lines
+13
to
+22
| it('locale switcher trigger meets 40px tap target height', function () { | ||
| $html = $this->get(route('feed'))->content(); | ||
|
|
||
| $triggerPos = strpos($html, 'locale-switcher-trigger'); | ||
| expect($triggerPos)->not->toBeFalse('locale-switcher-trigger not found'); | ||
|
|
||
| // Check that h-10 (40px) appears within the trigger element's markup | ||
| $snippet = substr($html, max(0, $triggerPos - 300), 500); | ||
| expect($snippet)->toContain('h-10'); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Хрупкая логика проверки высоты tap target через substring.
Тест использует strpos и substr для поиска класса h-10 в окрестности locale-switcher-trigger:
$snippet = substr($html, max(0, $triggerPos - 300), 500);
expect($snippet)->toContain('h-10');Проблемы:
- Ложноположительные срабатывания: тест пройдёт, если
h-10встречается где угодно в окне ±300 символов от триггера, даже если этот класс принадлежит другому элементу. - Хрупкость к изменению разметки: если в радиусе 300 символов появится другой элемент с
h-10, тест будет проходить некорректно.
Рекомендуется использовать более точный подход:
- Парсинг HTML через
DOMDocument/Symfony\Component\DomCrawler\Crawlerдля извлечения конкретного элемента по data-testid - Проверка класса непосредственно у
locale-switcher-trigger - Или использование CSS-селектора:
[data-testid="locale-switcher-trigger"].h-10
♻️ Рекомендуемое решение с использованием Crawler
it('locale switcher trigger meets 40px tap target height', function () {
- $html = $this->get(route('feed'))->content();
+ $response = $this->get(route('feed'));
+ $response->assertOk();
- $triggerPos = strpos($html, 'locale-switcher-trigger');
- expect($triggerPos)->not->toBeFalse('locale-switcher-trigger not found');
-
- // Check that h-10 (40px) appears within the trigger element's markup
- $snippet = substr($html, max(0, $triggerPos - 300), 500);
- expect($snippet)->toContain('h-10');
+ $crawler = new \Symfony\Component\DomCrawler\Crawler($response->content());
+ $trigger = $crawler->filter('[data-testid="locale-switcher-trigger"]');
+
+ expect($trigger->count())->toBeGreaterThan(0, 'locale-switcher-trigger not found');
+ expect($trigger->attr('class'))->toContain('h-10');
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Feature/Mobile/MobileSwitchersTest.php` around lines 13 - 22, The
current test uses strpos/substr ($triggerPos, $snippet) to search for 'h-10'
near 'locale-switcher-trigger', which is fragile and can yield false positives;
replace the substring approach by parsing the returned HTML ($html from
$this->get(route('feed'))) with a DOM parser/Crawler (e.g.,
Symfony\Component\DomCrawler\Crawler), locate the element by its data-testid or
selector for locale-switcher-trigger (e.g.,
'[data-testid="locale-switcher-trigger"]' or the element with class/ID used in
the markup), then assert directly that that element's class attribute contains
'h-10' (or that the element matches the selector
'[data-testid="locale-switcher-trigger"].h-10'); update the test to remove
$triggerPos/$snippet and use the Crawler-based lookup and assertion instead.
- Fix Tailwind v4 !-modifier syntax in rating-options (prefix → suffix) - Move mobile-header testid from empty sr-only span to app-header element - Move post-show-page testid from empty sr-only span to <main> element - Raise vendor/pagination height from 26px to 40px (match Livewire pagination) - Add language tag to fenced code block in mobile-ux-guidelines.md - Add inline comment explaining double rAF in feed-page scrollToSelectedPost - Add ->pause(500) in browser overflow tests before JS scroll measurement - Refactor viewport helper test to assert MobileViewports constants directly - Rename MobileFeedSpacingTest → MobileFeedStructureTest - Remove implementation-coupled space-y-4 assertion from upload form test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4 tasks
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.
Release v0.3.5 — Phase 48: Mobile UX Pass
Phase 48: Mobile UX (RG-755–RG-774)
min-w-0 overflow-hiddenoverflow prevention on flex/grid childrenbreak-wordson notification dropdown,truncateon profile display namedata-screenshotattributes on key pages for visual baseline testingPost-release fixes
overflow-hiddenon feed-page root div brokeposition: stickyon the detail panel; removed it and improvedscrollToSelectedPostto handle mobile (scroll to detail panel) and desktop (scroll to card, sticky panel stays visible). Added double-rAF for layout settle.PostFeedfrom->get()to->paginate(12)withWithPagination— eliminates full feed load (~5 MB). Custom page window: always shows first 3 and last 2 pages with ellipsis.WithPaginationuseslivewire::tailwindview (resources/views/vendor/livewire/tailwind.blade.php), notpagination::tailwind. Previous overrides had no effect. New view uses@once <style>with raw CSS variables for guaranteed theme-reactive colors andwire:clickfor in-place navigation.overflow-hiddenthat was clipping absolutely-positioned dropdown menus.--rg-shadow-dropdowntoken.🤖 Generated with Claude Code
Summary by cubic
Phase 48 delivers a mobile UX pass for v0.3.5 that removes 375px overflows, enforces 40px tap targets (incl. pagination), and converts the post drawer into a bottom sheet. It paginates the feed with
WithPaginationand theme-safelivewire::tailwind/pagination::tailwindviews, fixes the sticky post detail panel, and adds docs, a Mobile QA checklist, and viewport-based tests (RG-755–RG-774).New Features
min-w-0plusbreak-words/truncate; controls meet 40px tap targets; drawer becomes a bottom sheet on mobile.->paginate(12)withWithPagination; custom pagination inlivewire::tailwind/pagination::tailwinduses CSS variables for theme-safe colors, a compact window (first 3, current, last 2) with ellipsis, and 40px buttons.data-screenshottargets, Mobile QA checklist in the PR template, Mobile UX guidelines/audit/baselines, and new browser/feature tests with viewport helpers.Bug Fixes
scrollToSelectedPostwith double rAF and mobile vs. desktop scroll targets.overflow-hidden; added--rg-shadow-dropdown; notifications dropdown constrained tomax-w-[calc(100vw-2rem)]withbreak-words; ensuredmin-w-0on containers; header fits at 375px with truncated brand and abbreviated locale; locale trigger ish-10.!modifier syntax in rating options; standardized pagination buttons to 40px in bothlivewire::tailwindandpagination::tailwind; moved stable testids toapp-header/post-show-page; stabilized browser overflow tests with a 500ms pause.Written for commit 73c5b0c. Summary will update on new commits.
Summary by CodeRabbit
Улучшения мобильного интерфейса (Phase 48)
Новые возможности
Исправления
Документация