fix(notes): ניווט לפתק מגיע אליו — נגלל, נפתח, ועולה מעל השכנים

Merged
AM
1mo ago
+398-13

✨ תיאור קצר

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

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

  • קוד (Backend) — לא, פרונטאנד בלבד

  • בוט טלגרם

  • מסד נתונים/מיגרציות

  • תיעוד (docs/)

  • DevOps/CI/CD

  • הגלילה — scrollToNote עובר ל-scrollIntoView על אלמנט היעד.

  • פתק ממוזער נפתח, וההפתיחה נשמרת.

  • הפתק עולה לקדמת הערימה, תמיד.

  • תוצאת חיפוש נפתחת בכרטיסייה חדשה (target="_blank" + rel="noopener noreferrer").

  • צבע שתי השורות המשניות בכרטיס התוצאה נגזר מ---text-primary.

🔍 השורש, ושתי השערות שהמדידה הפריכה

מה נשבר. window.scrollTo מגלגל את document.scrollingElement, שהוא <html>. בעמוד הלוח נמדד בכרומיום שדווקא <body> הוא הגולל — scrollHeight 3758 מול clientHeight 800, בזמן ש-<html> כלוא על גובה המסך. כלומר הקריאה פנתה לאלמנט שאין לו לאן לזוז, ולכן שום דבר לא זז.

ההשערה הראשונה שלי הייתה שגויה. הנחתי ש-#boardSurface הוא הגולל, כי ה-CSS שלו מצהיר overflow: auto. המדידה הפריכה: המשטח גדל לגובה הפתק האחרון, ולכן scrollHeight == clientHeight ואין בו גלילה כלל. הצהרה על יכולת אינה עדות לשימוש.

וגם התיקון הראשון היה שגוי. כתבתי פותר שמחפש את הגולל בעצמו בשרשרת האבות. הוא עבד בבדיקה ידנית ונכשל בריצה האמיתית, כי בזמן שהוא רץ הפריסה עוד לא גדלה — התשובה הייתה נכונה לרגע הלא נכון.

הפתרון הוא לא לענות על השאלה. scrollIntoView מגלגל כל אב שנדרש, והדפדפן יודע מי הגולל בזמן אמת. הפותר הידני נמחק כולו, והשינוי יצא קטן יותר ממה שהחליף.

בנוסף, לולאת הניסיונות בודקת עכשיו תוצאה ולא קריאה: "קראנו ל-scrollToNote" אינו "הגענו לפתק", והמשטח גדל אחרי שהפתקים נטענו. _ensureInView מתקנת אם לא נחתנו — וחוזרת על הגלילה בלבד, כי scrollToNote פותחת פתק ומעלה אותו בערימה, ואלה פעולות חד-פעמיות.

🧪 בדיקות

  • Unit — 8 בדיקות JS חדשות, 7 מוטציות
  • Integration
  • Manual — מדידה בדפדפן עם ריצת בקרה

ריצת בקרה על הקוד שב-main, ולא על סטאב. הניסיון הראשון דרס את scrollToNote ב-prototype — והדריסה לא נכנסה לתוקף, כלומר מדדתי פעמיים את הקוד המתוקן וקיבלתי "עבר" פעמיים. בדיקה שאינה מסוגלת ליפול אינה ראיה. הבקרה האמיתית מגישה את הקובץ כפי שהוא ב-origin/main:

הפתק בתחום המסךtop בחלון (מתוך 800)ממוזערz
main✗3515כן950
אחרי✓557לא951

בדיקה קיימת תפסה תקלה אמיתית. tests/sticky-notes-deep-link.test.js סופר קריאות ל-scrollToNote, ונפל על 3 במקום 1 — כי גרסת הביניים חזרה על כל הפעולה בכל ניסיון. זה מה שהוביל להפרדה בין הפעולה לאימות. הבדיקה לא שונתה.

ניגודיות — וגם כאן המספר הראשון היה שגוי. המעבר מ---text-muted ל---text-secondary נראה נכון, ומדידה על שלוש הערכות המיובאות שבשימוש בפועל (CobaltNext, morass-Tech, pen-paper-coffee, דרך parse_vscode_theme האמיתי) הראתה שגם --text-secondary אינו אמין: בערכה שאינה מצהירה עליו הוא נופל לערך ברירת מחדל אפור ויוצא 2.84 מול רקע הכרטיס. הלקח: טוקן אחר הוא הימור, כי שום דבר אינו מחייב אותו להיות קריא דווקא על המשטח הזה. --text-primary כן מחויב, ושקיפות עליו מייצרת גוון משני שנגזר ממנו.

וערך השקיפות עצמו נמדד: הנמוכה מבין שלוש הערכות היא 6.01 במלא, 5.08 ב-0.88, 4.53 ב-0.80 ו-4.01 ב-0.72. הערך שנבחר הוא 0.88.

מה שלא נמדד: בערכת ברירת המחדל הטקסט בכרטיס יוצא 3.87 מול הרקע. זה --text-primary עצמו, כלומר מצב קיים של כרטיסים בכל האפליקציה ולא משהו שהעמוד הזה מייצר — לא נגעתי בו ולא חקרתי אותו.

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

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

📝 סוג שינוי

  • fix: תיקון באג

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון של הקבצים שהוא יושב בהם
  • בדיקות רצות ועוברות — כל בדיקות ה-JS וכל סוויטות הפתקים בפייתון
  • תיעוד עודכן — docs/user/sticky_notes.rst
  • לא נוספו ג'ובים חדשים, משתני סביבה או טוקנים חדשים
  • בדיקת rg "#[0-9a-fA-F]{3,6}" על notes-search.css מחזירה ריק
  • אין סודות/מפתחות בקוד, ואין מחיקות מסוכנות
  • עיינתי במסמכי אתר התיעוד — נתיב: docs/webapp/theming_and_css.rst | המשפט: "❌ אין לציין HEX בקבצי רכיבים (למעט חריגים מתועדים)"

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

רדיוס רחב מהחיפוש. scrollToNote משותפת ללוחות, לתצוגת המארקדאון ולדפדפן הריפו, והיא גם מה שקישור מתזכורת מפעיל. השינוי אמור לתקן את שלושתם — scrollIntoView פותר גם גולל מקונן — אבל נמדד רק על הלוח. המארקדאון ודפדפן הריפו לא נבדקו בדפדפן, כי שניהם דורשים תוכן מרונדר שאין לי בסביבה הזו.

שינוי נתונים אחד, מכוון: הגעה לפתק ממוזער כותבת is_minimized: false. זו הייתה החלטה מפורשת — פתק שנפתח לתצוגה בלבד היה חוזר להיות מכווץ בכניסה הבאה, ומי שלא מיזער אותו בעצמו היה מחפש ולא מוצא.

🔗 קישורים

  • ממשיך את #3408 (חיפוש בפתקים), שממנו עלו הפערים בשימוש בפועל
  • Issues פתוחים מהעבודה הקודמת: #3406, #3407

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

Revert מחזיר את scrollToNote להתנהגות הקודמת — כלומר את הבאג. הוא אינו מבטל את מה שכבר נשמר: פתקים שנפתחו יישארו פתוחים, וזה מצב תקין שהמשתמש יכול למזער בחזרה בכפתור.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PL3goRvy9ze5NrG5z68Awr

Generated by Claude Code

Summary by Sourcery

Make navigation to sticky notes reliably locate, reveal, and foreground the intended note while preserving the search workflow.

Bug Fixes:

  • Ensure deep links and navigation reliably bring the target note into view, reveal minimized notes, and place the selected note above overlapping neighbors.
  • Preserve the revealed state of minimized notes when they are opened through navigation.
  • Open note search results in a new, safer browser tab.
  • Improve the readability of secondary note-search metadata across supported themes.

Enhancements:

  • Separate one-time note reveal and stacking actions from repeated viewport correction during asynchronous layout changes.
  • Use element-based scrolling and viewport verification to support the actual scrolling container.

Documentation:

  • Update sticky-note user documentation to reflect the navigation behavior.

Tests:

  • Add unit coverage for revealing, persistence, stacking order, target selection, scrolling, and retry behavior.