fix(webapp): ציטוטים, רשימות וטבלאות מרונדרים ב-Live Preview של העורך - #3501
Conversation
כלל האיפוס `*` שב-base.html מאפס margin ו-padding לכל אלמנט, ועיצוב התוכן של md_preview.html יושב ב-<style> הפנימי שלו ואינו נטען בעמודי העריכה. לכן בתצוגה החיה ציטוט (`>`) נראה כמו פסקה רגילה, לרשימות לא היו תבליטים, לטבלאות לא היו קווים, ופסקאות נדבקו זו לזו. האדמוניציות לא נפגעו כי העיצוב שלהן ב-markdown-enhanced.css שנטען גלובלית. split-view.css מקבל בלוק תוכן Markdown משלו, מוגבל ל-canvas במצב markdown ועטוף ב-:where() כדי לגבור רק על האיפוס. הצבעים הם טוקנים חדשים (--split-preview-blockquote-*, --split-preview-table-*) שנגזרים ב-color-mix מטוקני התצוגה ולכן עובדים בכל ערכה; High Contrast דורס את הקווים לצבע הטקסט. בדיקת דפדפן חדשה בונה את עמוד העריכה מהתבניות האמיתיות ומרנדרת דרך כפתור ה-Live Preview בשבע ערכות. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Jsz4kD3wdGyd4t3QYEV4s
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's Guideהתיקון מוסיף שכבת CSS ממוקדת ל-Markdown בתצוגה החיה, כך שציטוטים, רשימות, טבלאות וריווח יוצגו כראוי בלי להשפיע על סטטוס התצוגה או על אדמוניציות; טוקנים נגזרים תומכים בכל הערכות, ובדיקות Playwright מאמתות את ההתנהגות בתבניות ובדפדפן האמיתיים. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughנוספו כללי CSS לעיצוב תוכן Markdown בתצוגה החיה, כולל ציטוטים, רשימות וטבלאות. נוספה בדיקת Playwright שבוחנת את העיצוב בשבע ערכות נושא. התיעוד מתאר את הכללים ואת בדיקת הדפדפן. Changesעיצוב Markdown בתצוגה החיה
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change adds scoped Markdown styling without changing preview rendering. No concrete merge-blocking issue is established; merge after normal checks pass. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 68.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 1 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="webapp/static/css/split-view.css" line_range="587-594" />
<code_context>
+ margin-block: 0.25rem 0.5rem;
+}
+
+:where(.split-preview-canvas[data-mode="markdown"]) table {
+ border-collapse: collapse;
+ margin: 0.75rem 0;
+ /* טבלה רחבה נגללת בתוך עצמה ולא מרחיבה את כל התצוגה */
+ display: block;
+ max-width: 100%;
+ overflow-x: auto;
+}
+
+:where(.split-preview-canvas[data-mode="markdown"]) :is(th, td) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Applying `display: block` directly to the `<table>` element removes its normal table formatting context. The table's row groups are laid out through anonymous table boxes, so columns between the header, body, and rows do not share a reliable table layout; wide tables also do not get the intended scrollable table behavior.
**Triggers:** When a Markdown table has multiple rows or columns, especially when its content is wider than the preview.
**Suggested fix:** Wrap the table in a dedicated overflow container, or preserve `display: table` on the table and apply horizontal scrolling to an outer wrapper.
</issue_to_address>
### Comment 2
<location path="tests/test_live_preview_blockquote_browser.py" line_range="49-52" />
<code_context>
+#: שלושת הדברים שבונים את מפל ה-CSS ב-``<head>`` של ``base.html``, לפי סדרם:
+#: בלוק ``<style>`` בלי מאפיינים (הבלוקים עם ``id`` מוזרקים לערכה מותאמת
+#: ומכילים Jinja), קישור ל-CSS מקומי, והמקום שבו נכנס ``extra_css``.
+HEAD_PART_RE = re.compile(
+ r"<style>(?P<style>.*?)</style>"
+ r"|<link rel=\"stylesheet\" href=\"\{\{ url_for\('static', filename='(?P<css>css/[^']+)'\) \}\}[^\"]*\">"
+ r"|(?P<extra>\{% block extra_css %\}\{% endblock %\})",
+ re.S,
+)
</code_context>
<issue_to_address>
**issue (testing):** The browser fixture claims to test the `custom` theme, but `HEAD_PART_RE` only extracts `<style>` elements without attributes and therefore omits the `style id="user-custom-theme"` block from `base.html`. The `page-custom.html` case consequently runs with the fallback `:root` tokens rather than the real custom-theme declarations, so custom-theme regressions can pass unnoticed.
**Triggers:** When the custom theme's CSS variables differ from the default variables.
**Suggested fix:** Include and render the attributed custom-theme style block in the fixture, or inject representative custom token values into the generated custom-theme page.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: webapp/static/css/split-view.css:594, tests/test_live_preview_blockquote_browser.py:52
⏱️ Performance report(No performance test durations collected. Mark tests with |
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| border-collapse: collapse; | ||
| margin: 0.75rem 0; | ||
| /* טבלה רחבה נגללת בתוך עצמה ולא מרחיבה את כל התצוגה */ | ||
| display: block; |
There was a problem hiding this comment.
WARNING: Applying display: block directly to the <table> element removes its normal table formatting context. The table's row groups are laid out through anonymous table boxes, so columns between the header, body, and rows do not share a reliable table layout; wide tables also do not get the intended scrollable table behavior.
Triggers: When a Markdown table has multiple rows or columns, especially when its content is wider than the preview.
Suggested fix: Wrap the table in a dedicated overflow container, or preserve display: table on the table and apply horizontal scrolling to an outer wrapper.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
נשאר כמו שהוא. זה אותו ממצא כמו של Sourcery, ושם יש את המדידה בדפדפן ואת ההפניה ל-CSS 2.1 §17.2.1: העמודות מיושרות, וטבלה רחבה נגללת בתוך עצמה.
Generated by Claude Code
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous Review Summary (commit 8091549)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8091549)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 0 · Output: 0 · Cached: 0 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
בגרסה הראשונה בלוק user-custom-theme של base.html לא רונדר (הוא Jinja), ולכן עמוד custom רץ על ערכי ה-fallback ולא בדק ערכה מותאמת בכלל (sourcery-ai / kilo-code-bot). עכשיו הבלוק האמיתי מרונדר עם משתנים שעברו את מסלול הייבוא של themes_api: parse_vscode_theme ואז validate_and_sanitize_theme_variables. בדיקה חדשה מוודאת שהעמוד אכן רץ על הצבעים שהוזרקו. ובתגובה לממצא על display: block בטבלה: נמדד שהעמודות מיושרות בין הכותרת לגוף (thead ו-tbody נעטפים יחד בטבלה אנונימית אחת, CSS 2.1 §17.2.1), ושטבלה רחבה נגללת בתוך עצמה. שתי בדיקות נועלות את זה. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Jsz4kD3wdGyd4t3QYEV4s
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Code Review ✅ Approved🟡 Medium risk · Live-preview Markdown styling changes layout and overflow behavior across themes. Fixes Markdown rendering in Live Preview by restoring styles for blockquotes, lists, tables, and paragraph spacing that were reset by the global OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
| #: ובלוק ``user-custom-theme`` — Jinja שמזריק את משתני הערכה המותאמת הפעילה. | ||
| #: שאר הבלוקים עם ``id`` (הדגשת תחביר, ערכה משותפת) אינם נוגעים בתצוגה החיה. | ||
| HEAD_PART_RE = re.compile( | ||
| r"<style>(?P<style>.*?)</style>" |
There was a problem hiding this comment.
CRITICAL: Regex alternation order bug — generic <style> pattern matches custom theme block first
The HEAD_PART_RE regex at lines 50-56 has the generic <style> pattern (line 51) before the custom theme pattern (line 52). Since regex alternation matches the first alternative that matches, the generic pattern will match the <style id="user-custom-theme"> block (which is also a <style>...</style> block).
Then in build_head() (lines 133-136), the assertion checks that the matched style block doesn't contain Jinja syntax ({{ or {%), but the custom theme block does contain Jinja (as seen in base.html:1869-1877). This will cause the assertion to fail.
The fix is to move the custom theme pattern BEFORE the generic style pattern in the alternation.
| r"<style>(?P<style>.*?)</style>" | |
| HEAD_PART_RE = re.compile( | |
| r"(?P<custom><style id=\"user-custom-theme\">.*?</style>)" | |
| r"|<style>(?P<style>.*?)</style>" | |
| r"|<link rel=\"stylesheet\" href=\"\{\{ url_for\('static', filename='(?P<css>css/[^']+)'\) \}\}[^"]*\">" | |
| r"|(?P<extra>\{% block extra_css %\}\{% endblock %\})", | |
| re.S, | |
| ) |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
לא משתנה. החלופה הכללית היא המחרוזת <style> כלשונה, עם > מיד אחרי style. לכן היא לא יכולה להתאים ל-<style id="user-custom-theme">, שבו אחרי style בא רווח ואז מאפיין. סדר החלופות לא משנה כאן, כי לא קיים טקסט שמתאים לשתיהן.
הרצתי את HEAD_PART_RE על ה-<head> של base.html:
- שורה 135: קבוצה
style - שורה 1380: קבוצה
style - שורה 1869: קבוצה
custom re.match(r"<style>", '<style id="user-custom-theme">')מחזירFalse
וגם בהרצה עצמה: אם התרחיש היה קורה, ה-assert שב-build_head היה מפיל את כל הקובץ. בפועל כל 47 הבדיקות עוברות, ו-test_the_custom_page_runs_on_the_injected_theme עובר רק כשהבלוק הזה מרונדר (במוטציה שמבטלת את ההזרקה הוא נופל).
הערה שולית: ההצעה עצמה לא הייתה עוברת קומפילציה. [^"]* בתוך r"..." סוגר את המחרוזת באמצע.
Generated by Claude Code
2 participants
✨ תיאור קצר
>) נראה בדיוק כמו פסקה רגילה. בבדיקה בדפדפן התברר שאותו שורש פגע גם ברשימות (אין תבליטים), בטבלאות (אין קווים) ובריווח בין פסקאות. האדמוניציות (::: note) היו תקינות.<blockquote>, אבל כלל האיפוס*שבבלוק ה-<style>שלbase.htmlמאפס margin ו-padding לכל אלמנט. עיצוב התוכן שלmd_preview.htmlיושב ב-<style>הפנימי שלו ולא נטען בעמודי העריכה, ולכן לא היה שום דבר שמחזיר את ההזחה, את הפס ואת הקווים. האדמוניציות עבדו רק כי העיצוב שלהן נמצא ב-markdown-enhanced.css, שנטען גלובלית.📦 שינויים עיקריים
פירוט:
webapp/static/css/split-view.css: בלוק "תוכן Markdown בתצוגה החיה" עם ציטוט (פס בתחילת השורה, רקע והזחה, כולל ציטוט מקונן), רשימות, טבלאות וריווח לפסקאות ולכותרות..split-preview-canvas[data-mode="markdown"], שמוגדר ב-setMarkdownContextשב-live-preview.js, כך ששורת הסטטוס ומציין המקום לא מושפעים.:where(), כך שהכללים גוברים רק על האיפוס, וכל כלל ממוקד יותר (למשל.admonition-content > *:first-child) ממשיך לנצח כמו היום.--split-preview-blockquote-*ו---split-preview-table-*. הם נגזרים ב-color-mixמ---split-preview-text/--split-preview-bgומוגדרים פעם אחת ב-:root, ולכן עובדים בכל ערכה (כולל מותאמת ומשותפת) בלי override לכל אחת. ב-High Contrast הקווים בצבע הטקסט המלא. אין HEX חדש.docs/webapp/theming_and_css.rstו-FEATURE_SUGGESTIONS/theme_matrix.md: שורה על שכבת התוכן ועל הטוקנים הנגזרים.🧪 בדיקות
tests/test_live_preview_blockquote_browser.py(Playwright + Chromium, 44 בדיקות).<style>וקישורי ה-CSS שלbase.htmlלפי הסדר,extra_css/extra_jsוה-DOM של התצוגה החיה מתוךedit_file.html.LivePreviewControllerהאמיתי (לחיצה על כפתור ה-Live Preview), בשבע ערכות: classic, dark, ocean, forest, rose-pine-dawn, high-contrast ו-custom.:where()מפילה אתtest_admonitions_keep_their_own_spacing, והרחבת הסקופ ל-.split-preview-contentמפילה אתtest_the_status_line_is_outside_the_markdown_scope.test_browser_suite_is_gated_once.py,test_size_format_consumers_browser.py,test_files_compact_view.py,test_mcp_outline.py,split-view-editor-height.test.jsו-sticky-notes-target.test.js.🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
docs/webapp/theming_and_css.rst+FEATURE_SUGGESTIONS/theme_matrix.mddocs/webapp/theming_and_css.rst| המשפט: "Split View ו‑Markdown Enhanced משתמשים ב‑--split-*ו‑--md-*בהתאמה – הוסיפו טוקן לפני שמוסיפים Class חדש." (עיינתי גם ב-AI-MAP.mdוב-docs/doc-authoring.rst)🧩 השפעות/סיכונים
edit_file.html,upload.html).md_preview.htmlלא נגע. תצוגת HTML רצה ב-iframe ולא מושפעת.🧯 סיכון / החזרה לאחור (Rollback)
🤖 Generated with Claude Code
https://claude.ai/code/session_018Jsz4kD3wdGyd4t3QYEV4s
Generated by Claude Code
Summary by Sourcery
Restore complete Markdown content styling in Live Preview across all supported themes.
Bug Fixes:
Enhancements:
Documentation:
Tests: