Skip to content

fix(webapp): תפריטי הסרגל העליון לחיצים שוב, וסרגל החיפוש נצמד רק בזמן חיפוש - #3536

Merged
amirbiron merged 2 commits into
mainfrom
claude/nifty-dijkstra-arcc1c
Oct 6, 2026
Merged

amirbiron merged 2 commits into
mainfrom
claude/nifty-dijkstra-arcc1c

Conversation

@amirbiron

@amirbiron amirbiron commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

תבנית Pull Request

✨ תיאור קצר

📦 שינויים עיקריים

  • קוד: תבניות ה-WebApp, ב-CSS בלבד, בלי JS ובלי שרת
  • בוט טלגרם
  • מסד נתונים/מיגרציות
  • תיעוד (docs/)
  • DevOps/CI/CD

פירוט נקודות:

  • base.html, שכבת הסרגל:
    • ה-z-index: 1000 של .navbar היה מת. z-index פועל רק על אלמנט ממוקם, והסרגל הוא static מאז Make header sticky on scroll #559. הסרתי אותו מהכלל הבסיסי.
    • נוסף .navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) { position: relative; z-index: 1000 }. כלומר הסרגל עולה מעל התוכן רק בזמן שאחד התפריטים שלו פתוח.
    • למה לא לתמיד: מדדתי שהרמה קבועה מסתירה בראש עמוד קובץ את 🔖 (#toggleBookmarksBtn) ואת כותרת פאנל הסימניות, ומניחה את הסרגל מעל מציג התמונות. בעשרות חלונות קופצים שיושבים בתוך .main-content הייתה קורה אותה בעיה.
  • base.html, חלונית האפקטים:
    • היא מעוגנת עכשיו ב-left: 0 ולא ב-right: 0. השרביט הוא הפריט השמאלי בסרגל, ולכן החלונית נפתחה אל מחוץ למסך.
    • צד לוגי (inset-inline-end) לא עוזר כאן. לחלונית יש direction: ltr משלה, ולכן צד לוגי מתורגם אצלה בחזרה ל-right, וזה נמדד.
  • base.html, פריסת ההמבורגר: .nav-menu.active { overflow: visible }. הרשימה הפתוחה חתכה את חלונית האפקטים, וכך היה גם לפני fix(webapp): גלילה וחיפוש בתצוגת קובץ — סרגל החיפוש נשאר על המסך, ▼ במתקדם מגיע לתוצאה, ומסך מלא נגלל עד הסוף #3535. החיתוך נשאר לאנימציית הקיפול.
  • view_file.html ו-md_preview.html:
    • ההצמדה עברה לכללים .editor-toolbar:has(#codeSearchInput:not(:placeholder-shown)) ו-.md-editor-toolbar:has(#mdSearchInput:not(:placeholder-shown)).
    • הרווח מעל כותרות Markdown (scroll-margin-top) חל רק באותו תנאי. בלי חיפוש, כותרת נוחתת בראש המסך.
  • מה לא השתנה: הסרגל העליון עצמו. בלי תפריט פתוח הוא נגלל עם העמוד, כמו מאז Make header sticky on scroll #559.

🧪 בדיקות

  • Unit: tests/test_navbar_and_search_bars_css.py, שומר טקסטואלי שרץ ב-CI.
    • הוא קורא את ה-CSS של שלוש התבניות ומוודא שההחלטות לא בוטלו.
    • יש בו 10 שינויים מכוונים בקוד, וכל אחד מהם מפיל אותו. ביניהם הרמה קבועה של הסרגל, עיגון החלונית לימין או לצד לוגי, ותיבה בלי placeholder.
    • 18 עוברות. על main נכשלות 7 בדיקות ההחלטה.
    • פרסור משותף: הפרסור של כללי ה-CSS יושב ב-tests/_css_rules.py, ומשמש גם את tests/test_sticky_notes_touch_action.py. השומר החדש שכפל אותו מהקובץ ההוא, ולכן איחדתי (R6). כל הבדיקות של שומר הפתקים, כולל המוטציות שלו, עוברות.
  • Integration: בדיקות דפדפן בכרומיום אמיתי, מול השרת האמיתי ומונגו אמיתי, בנגיעות CDP אמיתיות. לאן הגיעה כל נגיעה נבדק לפי האירוע עצמו, ולא לפי elementFromPoint.
    • tests/test_navbar_menus_browser.py (חדש):
      • נגיעה בכל כפתור בשני התפריטים, בטאבלט לרוחב ובפריסת ההמבורגר.
      • בדיקת שמירה: אחרי שתפריט נסגר, 🔖 וכותרת הפאנל עדיין לחיצים.
    • tests/test_view_file_scroll_browser.py:
      • בדיקה חדשה: הסרגל צמוד רק עם טקסט, ו-✕ משחרר אותו, בתצוגה הבסיסית ובמתקדמת.
      • בדיקת ה-Markdown עודכנה להתנהגות החדשה, בטאבלט ובטלפון.
    • ריצת בקרה:
      • עם התיקון כל 13 הבדיקות עוברות, בשתי ריצות.
      • על main נכשלות 8 מהן, מהסיבה הנכונה. למשל: הקיצור הראשון לא לחיץ, כפתור האפקטים ב-x=-91 מחוץ למסך, והסרגל צמוד ב-0 בלי טקסט.
      • על גרסת הרמה קבועה רק בדיקת השמירה נכשלת: 🔖 לא לחיץ.
    • בדיקות קיימות: עוברות 862 בדיקות שקוראות את התבניות, 1848 בדיקות שקוראות את התיעוד, ו-26 בדיקות דפדפן, כולל test_repo_notes_browser.py שמשתמש באותו harness.
    • לינט: flake8 (גם הבחירה החוסמת של ה-CI) ו-mypy נקיים על קובצי הבדיקות.
  • Manual: לא נבדק במכשיר אנדרואיד אמיתי. זו אמולציית מגע בכרומיום. גם Telegram Mini App לא נבדק.

🧪 בדיקות נדרשות ב‑PR

  • 🔍 Code Quality & Security
  • Unit Tests (3.11)
  • Unit Tests (3.12)
  • Unit Tests md-heavy (3.11)
  • Unit Tests md-heavy (3.12)

📝 סוג שינוי

  • feat: פיצ'ר חדש
  • fix: תיקון באג
  • docs: שינוי תיעוד בלבד
  • refactor: שינוי קוד ללא שינוי התנהגות
  • perf: שיפור ביצועים
  • chore/ci: תשתית/CI
  • breaking change: שינוי שובר תאימות

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון (Black/isort/flake8/mypy)
  • בדיקות רצות ועוברות
  • תיעוד עודכן (README/Docs). ב-docs/whats-new.rst נוספה רשומה. ב-docs/webapp/theming_and_css.rst נוספו שני כללים: z-index על אלמנט שאינו ממוקם והקשר ערימה שכולא את הילדים, והצמדה רק בזמן חיפוש. בנוסף עודכן כלל ה-clip: המעבר משחרר גם רכיבים צפים שהיו חתוכים.
  • אם נוספו ג'ובים חדשים: לא רלוונטי
  • אם נוספו/שונו משתני סביבה: לא רלוונטי
  • אם נוספו/השתנו טוקנים: לא רלוונטי. השינוי לא נוגע בטוקני צבע.
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות/פעולות על root
  • הודעת הקומיט תואמת Conventional Commits
  • CHANGELOG עודכן אם נדרש (whats-new)
  • כל ה‑Required Checks לעיל ירוקים (ממתין ל-CI)
  • צילום/וידאו UI מצורף אם רלוונטי
  • עיינתי במסמכי אתר התיעוד. נתיב: docs/webapp/theming_and_css.rst. המשפט: "clip חותך לרוחב בדיוק אותו דבר בלי להפוך את המיכל לקופסת גלילה". עליו נשען fix(webapp): גלילה וחיפוש בתצוגת קובץ — סרגל החיפוש נשאר על המסך, ▼ במתקדם מגיע לתוצאה, ומסך מלא נגלל עד הסוף #3535, וממנו נגזר הפספוס שתוקן כאן.
  • עוד עמודים שעיינתי בהם:
    • AI-MAP.md: בדקתי, ואין עמוד על הסרגל העליון או על התפריטים שלו.
    • docs/doc-authoring.rst: בלי ספירות מופעים בפרוזה.
    • docs/versioning-stable-anchors.rst: לא נשבר אף עוגן.
    • docs/testing.rst: בדיקות דפדפן מדולגות ב-CI.
  • דפוסי באגים שקראתי:
    • H7 ו-rtl-geometry-and-clamp
    • browser-policy, כולו
    • prose-restates-code-fact ו-line-number-coupling
    • claude-md-snippets/testing.md ו-T4
    • R6
    • linter-fix-changes-runtime-behavior
    • K15 ו-lazy-init-guard-publish-order. הלקח "התנהגות מנוונת שקיימת מזמן היא לעיתים תשתית שמישהו נשען עליה — מודדים מי" הוא מה שהוביל למדידה שפסלה את ההרמה הקבועה.

🧩 השפעות/סיכונים

  • בזמן שתפריט של הסרגל פתוח: רכיב צף שעולה על הסרגל (כמו 🔖 בראש עמוד קובץ) מוסתר מתחתיו עד שהתפריט נסגר. זה צפוי.
  • בסגירת תפריט: החלק של החלונית שמעל התוכן נעלם מיד ולא דוהה במשך רבע שנייה. השהיה של הירידה הייתה משאירה את החלונית הנסגרת לחיצה מעל התוכן.
  • טלפון קטן לרוחב: שם רשימת ההמבורגר ארוכה מ-70% מגובה המסך. סוף הרשימה, שהיה חתוך ולא נגיש, ייצא עכשיו מעל התוכן, בלי רקע. לא מדדתי את המקרה הזה.
  • דפדפן בלי :has(): הכללים החדשים לא חלים. התפריטים מתנהגים כמו ב-main, והסרגלים לא נצמדים.

🔗 קישורים

🧯 סיכון / החזרה לאחור (Rollback)

  • revert לשני הקומיטים. התפריטים יחזרו להיות לא לחיצים, והסרגלים יחזרו להיות צמודים תמיד, כמו ב-main היום.

סטיות מהתוכנית שאושרה

  • ריכוז הקוד המשותף לבדיקות: התוכנית אמרה להעביר את tablet_page כולה ל-tests/_browser_harness.py. בפועל עברו רק הקמת העמוד (open_admin_page) ו-tap_on. הסיבה היא ש-tests/test_browser_suite_is_gated_once.py אוסר להרים דפדפן מחוץ לקובצי *_browser.py, ולכן כל קובץ דפדפן ממשיך להרים את הדפדפן בעצמו.
  • שומר טקסטואלי ל-CI: לא היה בתוכנית, והוספתי אותו. בדיקות הדפדפן מדולגות ב-CI, ובלעדיו שום דבר ב-CI לא היה שומר על ההחלטות.
  • פרסור CSS משותף (tests/_css_rules.py): לא היה בתוכנית. השומר החדש שכפל את הפרסור של tests/test_sticky_notes_touch_action.py, ולכן איחדתי (R6), בקומיט נפרד.
  • בדיקת הסרגל בתצוגת קובץ: אחרי ההקלדה העמוד נגלל בקוד, ולא דרך הקפיצה לתוצאה. הסיבה בממצא הצדדי הראשון.

ממצאים צדדיים, לא תוקנו כאן

🤖 Generated with Claude Code

https://claude.ai/code/session_013Pzf4XyLpo6NFzLyZVnWxm


Generated by Claude Code

<source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"> <source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img alt="Turn on auto-fix" src="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg">

Summary by Sourcery

Fix webapp navigation menu touch targets and make search toolbars sticky only during active searches.

Bug Fixes:

  • Restore touch interaction for the top navigation quick-access and effects menus across desktop and hamburger layouts.
  • Make file and Markdown search toolbars sticky only while their search fields contain text, allowing them to scroll normally otherwise.

Enhancements:

  • Ensure open navigation menus render above page content without permanently obscuring floating controls.
  • Improve effects-menu positioning and hamburger-menu overflow so dropdown content remains visible and accessible.

Documentation:

  • Document the stacking and conditional sticky-toolbar behavior in the webapp CSS guidance and release notes.

Tests:

  • Add CSS guards that verify the navigation layering, dropdown positioning, overflow, and conditional search-toolbar rules.
  • Add Chromium touch tests covering navigation menu interactions, floating controls, and search-toolbar behavior across file-view modes and Markdown viewport sizes.

…ן חיפוש

אחרי #3535, חלונית הקיצורים ותפריט האפקטים בסרגל העליון נפתחו אבל לא הגיבו לנגיעה. הסרגל (.navbar) יושב בשכבה שמתחת ל-.main-content: ה-z-index שלו מת כי הוא static, וה-transform וה-backdrop-filter שלו כולאים בתוכו את החלוניות. מאז #3535 החלוניות כבר לא נחתכות, ולכן הן יוצאות אל מעל התוכן, והנגיעה הולכת לתוכן.

עכשיו הסרגל עולה מעל התוכן רק בזמן שאחד התפריטים שלו פתוח (.navbar:has(...)), ולא לתמיד: נמדד שהרמה קבועה מסתירה את 🔖, את כותרת פאנל הסימניות ואת החלונות שנפתחים מתוך התוכן. חלונית האפקטים מעוגנת לצד השמאלי של השרביט, ורשימת ההמבורגר הפתוחה כבר לא חותכת אותה. סרגלי החיפוש בתצוגת קובץ וב-Markdown נצמדים רק כשיש טקסט בתיבה, וגם הרווח מעל כותרות Markdown חל רק אז.

בדיקות: tests/test_navbar_menus_browser.py (חדש), עדכון של tests/test_view_file_scroll_browser.py, ושומר טקסטואלי ל-CI ב-tests/test_navbar_and_search_bars_css.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Pzf4XyLpo6NFzLyZVnWxm
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@sourcery-ai

sourcery-ai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Sorry @amirbiron, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 23 hours and 15 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

🧯 Dangerous deletes guard report

Policy: see .cursorrules — dangerous deletions are blocked unless wrapped safely.

Summary:

  • Flagged findings (blocking): 0
    0
  • Excluded matches (not blocking): 15
  • Total matches (all files): 128

Flagged findings (file:line:snippet):
(none)

Excluded matches (by path pattern)
./node_modules/katex/src/fonts/Makefile:139:	rm -rf pfa ff otf ttf woff woff2
./node_modules/katex/package.json:153:    "build": "rimraf dist/ && mkdirp dist && cp README.md dist && rollup -c --failAfterWarnings && webpack && node update-sri.js package dist/README.md",
./node_modules/mermaid/dist/mermaid.js.map:4:  "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values for tr … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm/chunk-2M32CCKP.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence d … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.core/chunk-KS23V3DP.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence  … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequen … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs:1:var r={name:"mermaid",version:"11.12.0",description:"Markdown-ish syntax for generating flowcharts, mindmaps, sequence diagrams, class diagrams, gantt charts, git graph … [truncated]
./node_modules/mermaid/dist/mermaid.min.js:1524:`,"getStyles"),c1e=RQe});var h1e={};dr(h1e,{diagram:()=>NQe});var NQe,f1e=N(()=>{"use strict";$ge();a1e();l1e();u1e();NQe={parser:Fge,db:n1e,renderer:o1e,styles:c1e}});var m1e,g1e=N(()=>{"use  … [truncated]
./node_modules/mermaid/dist/mermaid.min.js.map:4:  "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values fo … [truncated]
./README.md:829:find . -name "__pycache__" -exec rm -rf {} +
./webapp/static/js/md_preview.bundle.js.map:4:  "sourcesContent": ["// Markdown-it plugin to render GitHub-style task lists; see\n//\n// https://github.com/blog/1375-task-lists-in-gfm-issues-pulls-comments\n// https://github.com/blog/1825-t … [truncated]
./Dockerfile:42:    rm -rf /var/lib/apt/lists/*
./Dockerfile:121:    rm -rf /var/lib/apt/lists/*
./docs/DOCUMENTATION_GUIDE.md:453:rm -rf _build
./docs/Makefile:24:	rm -rf $(BUILDDIR)

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7feab86b-b621-4b39-ab9f-231ff08f5b39
📥 Commits

Reviewing files that changed from the base of the PR and between e0b1644 and b706cc1.

📒 Files selected for processing (11)
  • docs/webapp/theming_and_css.rst
  • docs/whats-new.rst
  • tests/_browser_harness.py
  • tests/_css_rules.py
  • tests/test_navbar_and_search_bars_css.py
  • tests/test_navbar_menus_browser.py
  • tests/test_sticky_notes_touch_action.py
  • tests/test_view_file_scroll_browser.py
  • webapp/templates/base.html
  • webapp/templates/md_preview.html
  • webapp/templates/view_file.html

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

תבניות ה-WebApp עודכנו כדי להציג תפריטי ניווט ללא חיתוך ולהעלות את סרגל הניווט מעל התוכן רק כאשר תפריט פתוח. סרגלי החיפוש בתצוגות קובץ ו-Markdown נדבקים רק בזמן חיפוש. נוספו בדיקות CSS ודפדפן לתנאים אלה.

Changes

סרגלי ניווט וחיפוש

שכבה / קבצים סיכום
כלי בדיקה ותפריטי ניווט
tests/_browser_harness.py, tests/_css_rules.py, webapp/templates/base.html, tests/test_navbar_and_search_bars_css.py, tests/test_navbar_menus_browser.py
נוספו כלי עזר משותפים לניתוח CSS ולבדיקות מגע בדפדפן. סרגל הניווט מקבל z-index רק כאשר אחד מתפריטיו פעיל. התפריט הפתוח במובייל מאפשר גלישה מחוץ לגבולותיו, וחלונית Fun Mode מעוגנת לשמאל. נוספו בדיקות CSS ודפדפן לתפריטים.
הצמדה מותנית של סרגלי חיפוש
webapp/templates/md_preview.html, webapp/templates/view_file.html, tests/test_view_file_scroll_browser.py, tests/test_navbar_and_search_bars_css.py, tests/test_sticky_notes_touch_action.py, docs/webapp/theming_and_css.rst, docs/whats-new.rst
סרגלי החיפוש נדבקים רק כשהשדה מכיל טקסט. בתצוגת Markdown, רווח הגלילה מעל כותרות ועוגנים חל רק בזמן חיפוש. בדיקות הדפדפן בודקות את המצבים עם חיפוש ובלעדיו. בדיקת touch-action משתמשת כעת בפרסר CSS המשותף. התיעוד ויומן השינויים עודכנו.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b706c

This change restores touch interaction for the top-bar menus and makes search toolbars sticky only while searching. The changes are limited to CSS and tests, and no concrete merge-blocking issue was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 6 files. (5 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed הכותרת מתארת בבירור את תיקון תפריטי הסרגל ואת הצמדת סרגל החיפוש רק בזמן חיפוש.
Description check ✅ Passed התיאור מפורט ותואם למבנה התבנית. הוא מסביר את השינוי, הבדיקות, הסיכונים ותוכנית החזרה לאחור, ומציין שבדיקות CI עדיין ממתינות ושבדיקות במכשיר Android וב-Telegram Mini App לא בוצעו.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 77.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 6 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

כשהתפריט נפתח, הסרגל עולה,
והרשימה גולשת בלי להיחתך.
החיפוש מדליק הצמדה בזמן,
ובניקוי השדה היא נעלמת.
בדיקות מגע שומרות על המסלול,
מחיאת כפיים ל-Claude Code על מלאכת הקוד!

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Fixes navbar menu touch handling by conditionally raising the navbar and correcting dropdown overflow/positioning, while changing file and Markdown search toolbars to stick only during active searches; adds browser and CI-safe CSS regression tests covering the responsive interactions and scrolling behavior.

Sequence diagram for conditional navbar menu interaction

sequenceDiagram
    actor User
    participant Navbar
    participant MainContent
    participant Menu
    User->>Menu: tap menu button
    Menu->>Navbar: activate menu
    Navbar->>Navbar: apply :has(...active) rule
    Navbar->>MainContent: render above content
    User->>Menu: tap dropdown item
    Menu-->>User: receive touch
    User->>Menu: close menu
    Navbar->>MainContent: return below content
Loading

Flow diagram for search toolbar sticky behavior

flowchart TD
    A[Search input state] --> B{Input has text?}
    B -->|Yes| C["Apply :has(:not(:placeholder-shown))"]
    C --> D[Toolbar position: sticky]
    D --> E[Headings use scroll-margin-top]
    B -->|No| F[Toolbar scrolls with document]
    F --> G[Headings have no search offset]
Loading

File-Level Changes

Change Details Files
Elevate the navbar only while its menus are open and make the effects menu render within the viewport and outside the hamburger clipping region.
  • Add a conditional :has(...) stacking rule that positions the navbar above .main-content only for active menus.
  • Anchor the effects dropdown to the left edge and allow an open hamburger menu to overflow visibly.
  • Remove the ineffective always-on navbar z-index to preserve clickability of content controls when menus are closed.
webapp/templates/base.html
Make file and Markdown search toolbars sticky only when their search fields contain text, and conditionally offset Markdown scroll targets.
  • Gate sticky positioning with :has(input:not(:placeholder-shown)) in both templates.
  • Apply Markdown heading and anchor scroll margins only while the search toolbar is pinned.
  • Document the stacking, clipping, and search-toolbar behavior changes.
webapp/templates/view_file.html
webapp/templates/md_preview.html
docs/webapp/theming_and_css.rst
docs/whats-new.rst
Add regression coverage for touch hit-testing, responsive layouts, search-toolbar scrolling, and CSS design invariants.
  • Centralize browser page setup and real-touch helpers while keeping browser startup in browser test modules.
  • Test every quick-access and effects menu item in desktop and hamburger layouts, plus bookmark interactions after menu closure.
  • Test toolbar scrolling with empty, populated, and cleared search fields in basic, advanced, tablet, phone, and Markdown views.
  • Add mutation-backed CSS guards that fail when key stacking, anchoring, overflow, placeholder, or conditional-sticky decisions regress.
tests/_browser_harness.py
tests/test_navbar_and_search_bars_css.py
tests/test_navbar_menus_browser.py
tests/test_view_file_scroll_browser.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

⏱️ Performance report

(No performance test durations collected. Mark tests with @pytest.mark.performance.)

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

The documentation has been built successfully!

To view locally:

  1. Download the artifacts
  2. Extract the zip file
  3. Open index.html in your browser

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mergestorm-vortex mergestorm-vortex Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review progress ██████████ 9/9 files

Approve — reviewed files look good at 2436223.

✅ All clear — nothing to fix.


Try MergeStorm

השומר החדש tests/test_navbar_and_search_bars_css.py שכפל את הפרסור של כללי CSS מ-tests/test_sticky_notes_touch_action.py. זה עותק שני של אותו כלל (R6). עכשיו שניהם קוראים דרך tests/_css_rules.py. הקובץ מסיר הערות, מפרק רשימת בוררים בפסיקים שמחוץ לסוגריים (:has(a, b) הוא בורר אחד), ושולף את בלוקי ה-<style> מתבנית בלי Jinja. ההתנהגות של שומר הפתקים לא השתנתה: כל הבדיקות שלו, כולל המוטציות, עוברות.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Pzf4XyLpo6NFzLyZVnWxm

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery assessment

Approved.

@gitar-bot

gitar-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

🟡 Medium risk · CSS changes alter navigation layering and when file/Markdown search bars stick.

Restores top navigation menu interaction and makes search toolbars sticky only during active searches. The navbar now appears above content only when menus are open, preventing permanent coverage of floating controls like bookmarks. CSS adjustments to z-index stacking, element anchoring, and conditional sticky positioning fix the regression from #3535. Comprehensive browser and unit test coverage validates menu touch targets, floating control accessibility, and toolbar scroll behavior across layouts. No issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="webapp/templates/base.html">

<violation number="1" location="webapp/templates/base.html:508">
P2: Custom agent: **Enforce Strict Maintainability Standards**

This rule hardcodes every supported dropdown class into the navbar stacking behavior, so a new floating menu silently regresses unless someone remembers to edit this selector. Move the open-state contract to a shared navbar state/class (with a compatibility fallback) instead of maintaining a feature-specific `:has()` registry.</violation>

<violation number="2" location="webapp/templates/base.html:508">
P3: חומרה: נמוכה (2/10). בסגירה מסירים את `.active` מיד — הסרגל חוזר ל-`position: static` בלי z-index, אבל החלונית ממשיכה להתעמעם במשך 0.25s (`transition: all 0.25s ease`). בחלון הזה החלונית עוד נראית אבל כבר מצוירת מתחת ל-.main-content (שנמצא מעל לסרגל כשהסרגל מורם, כמתואר בתגובה), ולכן התוכן מהבהב מעל החלונית הנסגרת. ההצעה: לקצר את החיווי על החלוניות (למשל `transition: opacity 0.15s ease, visibility 0s 0.15s;` במקום `all 0.25s ease` ב-.quick-access-dropdown / .fun-mode-dropdown), או לדחות את הסרת ההרמה עד סוף הטרנזיציה.</violation>
</file>

<file name="webapp/templates/md_preview.html">

<violation number="1" location="webapp/templates/md_preview.html:635">
P2: Custom agent: **Enforce Strict Maintainability Standards**

The toolbar infers search activity from `:placeholder-shown` and duplicates that implicit condition for heading offsets and the parallel file template. Represent search activity with an explicit class/data state maintained by the search handler, then style both behaviors from that contract; otherwise a placeholder change or a browser without `:has()` silently changes or removes the layout behavior.</violation>
</file>

<file name="webapp/templates/view_file.html">

<violation number="1" location="webapp/templates/view_file.html:107">
P3: חומרה: נמוכה (2/10). הגישה מתועדת בתגובה (בלי `:has()` הסרגל פשוט לא נצמד), אבל כדאי להיות מודעים לכך שזה נסיגה תפקודית מלאה בדפדפנים ישנים: לפני השינוי הסרגל היה דביק תמיד, ועכשיו בדפדפן ללא תמיכה ב-`:has()` (ספארי < 15.4, כרום < 105, פיירפוקס < 121 — וגם WebView אנדרואיד ישן, הרלוונטי ל-Telegram Mini App שלא נבדק לפי תיאור ה-PR) הסרגל מפסיק להיצמד לגמרי, וכך גם הרמת הסרגל העליון ב-base.html והקיזוז-גלילה ב-md_preview.html. אם הקהל כולל מכשירים כאלה, שיקול לזיהוי-תכונה ב-JS עם מחלקת fallback (למשל `html.has-has`) במקום לוותר בסתר על ההתנהגות.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

שהסרגל מתחתם, וסרגל שמורם תמיד מסתיר אותם בראש העמוד — נמדד.
המצב נקרא מה-.active שמציג כל תפריט, ולא מדגל נפרד שצריך לזכור לאפס בכל
דרך סגירה. תפריט צף חדש בסרגל נכנס לרשימה הזו. */
.navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Enforce Strict Maintainability Standards

This rule hardcodes every supported dropdown class into the navbar stacking behavior, so a new floating menu silently regresses unless someone remembers to edit this selector. Move the open-state contract to a shared navbar state/class (with a compatibility fallback) instead of maintaining a feature-specific :has() registry.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webapp/templates/base.html, line 508:

<comment>This rule hardcodes every supported dropdown class into the navbar stacking behavior, so a new floating menu silently regresses unless someone remembers to edit this selector. Move the open-state contract to a shared navbar state/class (with a compatibility fallback) instead of maintaining a feature-specific `:has()` registry.</comment>

<file context>
@@ -491,11 +491,25 @@
+           שהסרגל מתחתם, וסרגל שמורם תמיד מסתיר אותם בראש העמוד — נמדד.
+           המצב נקרא מה-.active שמציג כל תפריט, ולא מדגל נפרד שצריך לזכור לאפס בכל
+           דרך סגירה. תפריט צף חדש בסרגל נכנס לרשימה הזו. */
+        .navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) {
+            position: relative;
+            z-index: 1000;
</file context>


/* כמו .editor-toolbar ב-view_file.html: נצמד רק בזמן חיפוש — כשיש טקסט ב-#mdSearchInput,
שה-placeholder שלו הוא מה שמאפשר את התנאי — ובלי חיפוש נגלל עם המסמך. */
.md-editor-toolbar:has(#mdSearchInput:not(:placeholder-shown)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Enforce Strict Maintainability Standards

The toolbar infers search activity from :placeholder-shown and duplicates that implicit condition for heading offsets and the parallel file template. Represent search activity with an explicit class/data state maintained by the search handler, then style both behaviors from that contract; otherwise a placeholder change or a browser without :has() silently changes or removes the layout behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webapp/templates/md_preview.html, line 635:

<comment>The toolbar infers search activity from `:placeholder-shown` and duplicates that implicit condition for heading offsets and the parallel file template. Represent search activity with an explicit class/data state maintained by the search handler, then style both behaviors from that contract; otherwise a placeholder change or a browser without `:has()` silently changes or removes the layout behavior.</comment>

<file context>
@@ -628,6 +628,11 @@
+
+/* כמו .editor-toolbar ב-view_file.html: נצמד רק בזמן חיפוש — כשיש טקסט ב-#mdSearchInput,
+   שה-placeholder שלו הוא מה שמאפשר את התנאי — ובלי חיפוש נגלל עם המסמך. */
+.md-editor-toolbar:has(#mdSearchInput:not(:placeholder-shown)) {
   position: sticky;
   top: 0;
</file context>

שהסרגל מתחתם, וסרגל שמורם תמיד מסתיר אותם בראש העמוד — נמדד.
המצב נקרא מה-.active שמציג כל תפריט, ולא מדגל נפרד שצריך לזכור לאפס בכל
דרך סגירה. תפריט צף חדש בסרגל נכנס לרשימה הזו. */
.navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: חומרה: נמוכה (2/10). בסגירה מסירים את .active מיד — הסרגל חוזר ל-position: static בלי z-index, אבל החלונית ממשיכה להתעמעם במשך 0.25s (transition: all 0.25s ease). בחלון הזה החלונית עוד נראית אבל כבר מצוירת מתחת ל-.main-content (שנמצא מעל לסרגל כשהסרגל מורם, כמתואר בתגובה), ולכן התוכן מהבהב מעל החלונית הנסגרת. ההצעה: לקצר את החיווי על החלוניות (למשל transition: opacity 0.15s ease, visibility 0s 0.15s; במקום all 0.25s ease ב-.quick-access-dropdown / .fun-mode-dropdown), או לדחות את הסרת ההרמה עד סוף הטרנזיציה.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webapp/templates/base.html, line 508:

<comment>חומרה: נמוכה (2/10). בסגירה מסירים את `.active` מיד — הסרגל חוזר ל-`position: static` בלי z-index, אבל החלונית ממשיכה להתעמעם במשך 0.25s (`transition: all 0.25s ease`). בחלון הזה החלונית עוד נראית אבל כבר מצוירת מתחת ל-.main-content (שנמצא מעל לסרגל כשהסרגל מורם, כמתואר בתגובה), ולכן התוכן מהבהב מעל החלונית הנסגרת. ההצעה: לקצר את החיווי על החלוניות (למשל `transition: opacity 0.15s ease, visibility 0s 0.15s;` במקום `all 0.25s ease` ב-.quick-access-dropdown / .fun-mode-dropdown), או לדחות את הסרת ההרמה עד סוף הטרנזיציה.</comment>

<file context>
@@ -491,11 +491,25 @@
+           שהסרגל מתחתם, וסרגל שמורם תמיד מסתיר אותם בראש העמוד — נמדד.
+           המצב נקרא מה-.active שמציג כל תפריט, ולא מדגל נפרד שצריך לזכור לאפס בכל
+           דרך סגירה. תפריט צף חדש בסרגל נכנס לרשימה הזו. */
+        .navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) {
+            position: relative;
+            z-index: 1000;
</file context>

ה-placeholder של #codeSearchInput, כי הבורר :placeholder-shown מתקיים רק כשיש
placeholder והתיבה ריקה (גם כשמנקים אותה מקוד), ובלעדיו הסרגל היה נצמד תמיד.
דפדפן שלא מכיר את :has() מדלג על הכלל כולו, והסרגל פשוט לא נצמד. */
.editor-toolbar:has(#codeSearchInput:not(:placeholder-shown)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: חומרה: נמוכה (2/10). הגישה מתועדת בתגובה (בלי :has() הסרגל פשוט לא נצמד), אבל כדאי להיות מודעים לכך שזה נסיגה תפקודית מלאה בדפדפנים ישנים: לפני השינוי הסרגל היה דביק תמיד, ועכשיו בדפדפן ללא תמיכה ב-:has() (ספארי < 15.4, כרום < 105, פיירפוקס < 121 — וגם WebView אנדרואיד ישן, הרלוונטי ל-Telegram Mini App שלא נבדק לפי תיאור ה-PR) הסרגל מפסיק להיצמד לגמרי, וכך גם הרמת הסרגל העליון ב-base.html והקיזוז-גלילה ב-md_preview.html. אם הקהל כולל מכשירים כאלה, שיקול לזיהוי-תכונה ב-JS עם מחלקת fallback (למשל html.has-has) במקום לוותר בסתר על ההתנהגות.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webapp/templates/view_file.html, line 107:

<comment>חומרה: נמוכה (2/10). הגישה מתועדת בתגובה (בלי `:has()` הסרגל פשוט לא נצמד), אבל כדאי להיות מודעים לכך שזה נסיגה תפקודית מלאה בדפדפנים ישנים: לפני השינוי הסרגל היה דביק תמיד, ועכשיו בדפדפן ללא תמיכה ב-`:has()` (ספארי < 15.4, כרום < 105, פיירפוקס < 121 — וגם WebView אנדרואיד ישן, הרלוונטי ל-Telegram Mini App שלא נבדק לפי תיאור ה-PR) הסרגל מפסיק להיצמד לגמרי, וכך גם הרמת הסרגל העליון ב-base.html והקיזוז-גלילה ב-md_preview.html. אם הקהל כולל מכשירים כאלה, שיקול לזיהוי-תכונה ב-JS עם מחלקת fallback (למשל `html.has-has`) במקום לוותר בסתר על ההתנהגות.</comment>

<file context>
@@ -97,6 +97,14 @@
+   ה-placeholder של #codeSearchInput, כי הבורר :placeholder-shown מתקיים רק כשיש
+   placeholder והתיבה ריקה (גם כשמנקים אותה מקוד), ובלעדיו הסרגל היה נצמד תמיד.
+   דפדפן שלא מכיר את :has() מדלג על הכלל כולו, והסרגל פשוט לא נצמד. */
+.editor-toolbar:has(#codeSearchInput:not(:placeholder-shown)) {
     position: sticky;
     top: 0;
</file context>

@mergestorm-vortex mergestorm-vortex Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review progress ██████████ 11/11 files

Approve — reviewed files look good at b706cc1.

✅ All clear — nothing to fix.


Try MergeStorm

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 existing issues remain and no new issues found across 11 files

Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.

Turn on auto-fix | Re-trigger cubic

שהסרגל מתחתם, וסרגל שמורם תמיד מסתיר אותם בראש העמוד — נמדד.
המצב נקרא מה-.active שמציג כל תפריט, ולא מדגל נפרד שצריך לזכור לאפס בכל
דרך סגירה. תפריט צף חדש בסרגל נכנס לרשימה הזו. */
.navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Hardcoded dropdown classes in :has() selector reduces maintainability

The selector .navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) explicitly lists every dropdown class. When a new floating menu is added to the navbar, it will silently fail to raise the navbar unless someone remembers to update this selector. Consider using a shared state class (e.g., .navbar-menu-open) that each menu toggles on its container, or a data attribute on the navbar itself, so the CSS rule only needs to check for one condition.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

שהסרגל מתחתם, וסרגל שמורם תמיד מסתיר אותם בראש העמוד — נמדד.
המצב נקרא מה-.active שמציג כל תפריט, ולא מדגל נפרד שצריך לזכור לאפס בכל
דרך סגירה. תפריט צף חדש בסרגל נכנס לרשימה הזו. */
.navbar:has(.nav-menu.active, .quick-access-dropdown.active, .fun-mode-dropdown.active) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Transition timing causes content to flash over closing dropdown panels

When a menu closes, .active is removed immediately, causing the navbar to drop back to position: static (no z-index). However, the dropdown panels have transition: all 0.25s ease and remain visible (fading out) for 250ms. During this window, the panels render under .main-content (z-index: 1) because the navbar no longer creates a stacking context above it. This causes page content to visibly flash over the closing panel.

Fix options:

  1. Shorten panel transitions to transition: opacity 0.15s ease, visibility 0s 0.15s; (hides panel faster)
  2. Use transition-delay on the navbar's position/z-index change via JS (remove .active after transition ends)
  3. Keep position: relative on navbar until panel animations complete

Reply with @kilocode-bot fix it to have Kilo Code address this issue.


/* כמו .editor-toolbar ב-view_file.html: נצמד רק בזמן חיפוש — כשיש טקסט ב-#mdSearchInput,
שה-placeholder שלו הוא מה שמאפשר את התנאי — ובלי חיפוש נגלל עם המסמך. */
.md-editor-toolbar:has(#mdSearchInput:not(:placeholder-shown)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Implicit search state via :placeholder-shown duplicates logic and is fragile

The toolbar sticky behavior, heading scroll offsets, and the parallel file template (view_file.html) all infer "search is active" from #mdSearchInput:not(:placeholder-shown). This duplicates the condition across multiple selectors and templates. If the placeholder text changes or is removed, the sticky behavior breaks silently. A browser without :has() support loses the behavior entirely with no fallback.

Better approach: Add an explicit class (e.g., .md-editor-toolbar.search-active) or data attribute (e.g., data-search-active) toggled by the search input's JS handler. Then style both the toolbar pinning and heading offsets from that single contract.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

ה-placeholder של #codeSearchInput, כי הבורר :placeholder-shown מתקיים רק כשיש
placeholder והתיבה ריקה (גם כשמנקים אותה מקוד), ובלעדיו הסרגל היה נצמד תמיד.
דפדפן שלא מכיר את :has() מדלג על הכלל כולו, והסרגל פשוט לא נצמד. */
.editor-toolbar:has(#codeSearchInput:not(:placeholder-shown)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: :has() browser support gap causes complete loss of sticky search toolbar

The new conditional sticky behavior (.editor-toolbar:has(#codeSearchInput:not(:placeholder-shown))) requires :has() support. In browsers without it (Safari < 15.4, Chrome < 105, Firefox < 121, and older Android WebView — relevant for Telegram Mini App which PR description notes was not tested), the selector is invalid and the entire rule is dropped. The toolbar simply never sticks, whereas before this PR it was always sticky.

Consider a progressive enhancement: detect :has() support in JS (CSS.supports('selector(:has(*))')) and set a class on <html> (e.g., html.has-has) to gate the conditional rules, with a fallback to always-sticky for unsupported browsers.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
webapp/templates/base.html 508 Hardcoded dropdown classes in :has() selector reduces maintainability
webapp/templates/md_preview.html 635 Implicit search state via :placeholder-shown duplicates logic and is fragile

SUGGESTION

File Line Issue
webapp/templates/base.html 508 Transition timing causes content to flash over closing dropdown panels
webapp/templates/view_file.html 107 :has() browser support gap causes complete loss of sticky search toolbar
Files Reviewed (11 files)
  • webapp/templates/base.html - 2 issues
  • webapp/templates/md_preview.html - 1 issue
  • webapp/templates/view_file.html - 1 issue
  • tests/_browser_harness.py
  • tests/_css_rules.py
  • tests/test_navbar_and_search_bars_css.py
  • tests/test_navbar_menus_browser.py
  • tests/test_sticky_notes_touch_action.py
  • tests/test_view_file_scroll_browser.py
  • docs/webapp/theming_and_css.rst
  • docs/whats-new.rst

Fix these issues in Kilo Cloud


Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 715.7K · Output: 3.7K · Cached: 807.8K

@amirbiron
amirbiron merged commit c5df29e into main Oct 6, 2026
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants