Retro

PR #846 — комментарии и разбор

PR: https://bitbucket.org/stevens_edu/stevens_main_nextjs/pull-requests/846 Задача: IX-SIT67 — update header (parent ICUS-220)

Header прямо упомянут в письме Alexis как пример "deviate from spec". Этот PR — критический для разговора.

Таймлайн PR

ДатаСобытие
2026-04-16, 12:16 WarsawПриложен mockups v3.zip к ICUS-220
2026-04-20, 14:24 WarsawПриложен 📄Site_Header_User_Stories_v2.docx
2026-04-22 (ср)Иван открыл PR #846 + 1 коммит 7dd5f2b
2026-04-22 (ср)Vercel deploy ready (last update Apr 28, 11:40am UTC)
2026-04-29 (ср), 22:28Письмо Alexis Watson с упоминанием header как примера

Хронологически очень тонкий момент:

  • PR открыт 2026-04-22 — то есть 2 дня после стабилизации User Stories v2 (20.04) и 6 дней после mockups v3 (16.04).
  • Письмо Alexis — через 7 дней после открытия PR (29.04).

Это значит, что аргумент "спека итерировалась" очень ослаблен: к моменту открытия PR на ICUS-220 уже лежали финальные вложения по датам (v2/v3 в имени — не аргумент про «много черновиков» для нас). Иван открыл PR имея актуальные материалы с тикета.

НО — задача называется "update header", не "create header". То есть header в каком-то виде уже был в проде. Alexis в письме говорит "deliverables that deviate considerably from spec and require revisiting" → скорее всего прежний header (доставленный ранее, в другом, более раннем PR) — это тот самый "deviating deliverable", а PR #846 — это попытка revisiting, которую она и ждала.

Это меняет картину:

  • Конкретный PR #846 — не нарушитель, он лекарство.
  • "Преступник" — это предыдущая итерация header'а, которая уже была залита раньше (нужно найти тот PR).
  • PR #846 — наша попытка ответа: мы привести header к спеке.

К встрече надо:

  1. Найти, в каком PR / когда был залит предыдущий header (тот, на который Alexis жалуется).
  2. По нему — посмотреть, какая была спека на тот момент. Возможно, спеки вообще не было, или она была "v1", или были только частичные мокапы.
  3. Тогда позиция честная: первоначальный заход был с неполной/устаревшей спекой (в т.ч. старый header в проде) → в апреле на ICUS-220 появились финальные вложения по датам → мы открываем PR #846 для приведения в соответствие. (Номера v2/v3 в именах файлов не доказывают, сколько черновиков мы видели — нам передали итоговые файлы.)

Comment #1 — Vercel bot (служебное)

  • Скриншот:
  • Автор: Vercel for Bitbucket (под аккаунтом Michael Forbes)
  • Дата: 2026-04-22
  • Не претензия. Deploy preview готов.
  • Иван UPDATED 1 коммит 7dd5f2b 22.04.
  • Last deploy update 2026-04-28, 11:40am UTC — то есть на момент письма Alexis (29.04, 22:28) PR был ещё совсем свежий, ещё не отревьюенный по существу.

Comment #2 — Michael Forbes: "Info For" dropdown центрирован, но это не в спеке - Скриншоты:

  • (комментарий + сравнение)

  • (резолюция)

  • Автор: Michael Forbes

  • Дата: 2026-04-23 (на следующий день после открытия PR)

  • Цитата:

    The items within the "Info For" dropdown are left-justified before this change, but center-justified with this change. Personally I'm not opposed to the change, but was it specified? I don't see this in the mockup for this issue.

  • Что было: в дропдауне "Info For" пункты (Future Students, Current Students, Faculty and Staff, Parents and Families, High School Counselors, Media) были выровнены по левому краю.

  • Что стало в PR: по центру.

  • Что было в спеке: этого изменения нет. Michael смотрел mockups v3 — там нет указания центрировать dropdown items.

  • Суть замечания: Иван сделал изменение, которое не запрашивалось. Тон Michael мягкий ("Personally I'm not opposed to the change") — то есть это не критично эстетически, но процессно проблема: реализация отклоняется от спеки, добавляя то, чего там нет.

  • Реакция Ивана (2026-04-23):

    Resolved

    И запушил 5 коммитов bcd15c3, d8a2a49, aa53014, 29e02bd, cd91a3c в тот же день — скорее всего вернул выравнивание по левому краю + другие правки.

  • Обоснованно? Да. Это ровно тот паттерн, на который жалуется Alexis в письме — "deviate from spec".

  • Важный нюанс: Это обратная сторона проблемы со спекой. На прошлых PR (#837, #842, #832) Иван не выполнял части спеки. Здесь — наоборот, сделал то, чего в спеке нет. Оба варианта — отклонение от спеки. Оба требуют от ревьюера сравнивать построчно, что и почему сделано.

    Возможные причины:

    • Иван увидел/прочитал не тот источник (старые мокапы / Figma / другой экран), где центрирование было.
    • Иван перенёс стиль из другого места проекта (например, mobile меню, где видно на mockup v3, что список центрирован).
    • Иван это сделал по своей инициативе (так выглядит "красивее").

  • Вопрос Ивану: откуда взялось center-justify для "Info For" dropdown? Была ли это твоя инициатива, или ты видел это где-то в спеке/Figma?

  • Связь с письмом Alexis: прямая — этот PR опубликован 22.04, и уже на следующий день у Michael есть пример "deviate from spec". На 29.04 (письмо) этот сигнал у него на руках. Теоретически это может быть одним из конкретных кейсов, который Alexis имела в виду в письме (наряду с предыдущим header'ом).

Comment #3 — Michael Forbes: меню переносится на 2 строки на 1024–1091px - Скриншот:

  • Автор: Michael Forbes

  • Дата: 2026-04-23

  • Цитата:

    When the viewport width is between 1024px and 1091px, the menu items in the red bar take two lines of text, which is not specified and does not occur before adding the search button to the red bar. Can anything be adjusted to avoid this?

  • Суть:

    • Иван добавил иконку поиска в красную (primary nav) полосу.
    • Из-за этого на ширине viewport 1024–1091px пунктам меню (ACADEMICS / DISCOVER STEVENS / STUDENT LIFE / RESEARCH / ADMISSION & AID) не хватает места, и они переносятся на 2 строки.
    • До добавления иконки поиска такой регрессии не было.
    • Спека этого визуально не фиксирует, но и не предполагает двустрочного меню (мокапы показывают одну строку).

  • Реакция Ивана (2026-04-23):

    Resolved

    → исправил.

  • Обоснованно? Да. Это визуальная регрессия, внесённая самим PR. Должно было быть поймано при responsive-тестировании на промежуточных ширинах, не только на 1280/1440/1920 (которые есть в спеке).

  • Связь со спекой: спека прямо указывает 1280 / 1440 / 1920 px ("renders correctly at common desktop resolutions") — Иван формально проверил эти точки. Но между tablet и desktop breakpoint'ом (1024–1091) есть промежуточная зона, которую Иван не покрыл.

  • Связь с письмом Alexis: это классическое "internal validation pass" — то, что должна была поймать наша сторона до отправки на ревью клиенту. Резкоsponsive-проверка занимает 2-3 минуты в DevTools (rwd-режим, ползунок ширины). Не сделана.

  • Вопрос Ивану: проверял ли responsive-поведение в промежуточных ширинах перед открытием PR? Какие breakpoint'ы используешь при тестировании?

  • Что важно: уже второй комментарий за один день от Michael с реальной проблемой. PR открыт 22.04, на 23.04 — два замечания (центрирование dropdown + меню в две строки). Это говорит о том, что PR не прошёл нормальную self-проверку перед отправкой.

Comment #4 — Michael Forbes: text-only логотип в expanded mobile menu не реализован - Скриншоты:

  • — что в спеке (text-only "STEVENS / INSTITUTE OF TECHNOLOGY", без щита)

  • — что в feature branch (старый shield-logo)

  • — резолюция Ивана

  • Автор: Michael Forbes

  • Дата: 2026-04-23

  • Цитата:

    The specification shows that the expanded mobile menu should have a text-only Stevens logo in the top left corner, which is a change from the old behavior currently in production. However, this is not yet implemented.

  • Что в спеке (mockup v3): в раскрытом мобильном меню в левом верхнем углу — текстовый логотип (просто "STEVENS / INSTITUTE OF TECHNOLOGY"), без иконки щита.

  • Что в PR: старый shield-logo (с иконкой здания/щита и текстом).

  • Реакция Ивана (2026-04-23):

    Resolved

    → исправил.

  • Обоснованно? Да, абсолютно. Это прямое визуальное расхождение с мокапом v3, который был приложен 16.04 — за 6 дней до открытия PR (22.04). Спека на руках, мокап с конкретным изменением логотипа, не реализовано.

  • Это самый серьёзный из визуальных промахов в этом PR:

    • Не цвет / не пиксель / не отступ — это сменённый брендовый элемент (logo style).
    • Это явное изменение поведения относительно текущего прода ("a change from the old behavior currently in production"), которое спека прямо вводит — и которое Иван просто не сделал.
    • 4-я задача за 2 дня от Michael — все из категории "spec deviation".

  • Связь с письмом Alexis: прямое попадание. Это именно то, что Alexis имеет в виду под "deliverables that deviate considerably from spec". Текстовый логотип в expanded mobile — заметная визуально вещь, которую невозможно "не заметить" при тестировании. Если Иван открыл expanded mobile menu хоть раз и сравнил с мокапом — он бы это увидел.

  • Вопрос Ивану: открывал ли expanded mobile menu и сравнивал с мокапом v3 (Site Header v03 Mobile.png / Mobile Search.png) до открытия PR? Если открывал — почему пропустил logo? Если не открывал — это вопрос процесса self-QA.

Накопительный счёт за 23.04 (один день после открытия PR)

  • Comment #2: dropdown center-justified — не в спеке (deviation, сделано лишнее)
  • Comment #3: меню в 2 строки на 1024–1091px (regression от добавления search button)
  • Comment #4: text-only logo в expanded mobile не реализован (deviation, не сделано требуемое)

Три отклонения от спеки в одном PR — найдены клиентом за один день после открытия. Все три видны глазом при базовом QA (открыть страницу, потянуть RWD-ползунок, открыть mobile menu).

Это и есть тот самый "header" из письма Alexis.

Comment #5 — Michael Forbes: dark mode → недостаточный контраст на жёлтых CTA - Скриншот:

  • Автор: Michael Forbes

  • Дата: 2026-04-23

  • Цитата:

    When using dark mode (in Chrome, visit chrome://flags/#enable-force-dark) there is insufficient contrast on the new yellow buttons. Can the button text stay the same (dark) regardless of mode?

  • Что видно на скрине: жёлтые CTA-кнопки Request Info / Visit / Apply в Chrome force-dark mode — текст становится светлым на жёлтом, контраст недостаточный. На скрине текст еле читается.

  • Связь со спекой: спека прямо требует WCAG AA contrast (минимум 4.5:1):

    "Buttons meet WCAG AA color contrast requirements (minimum 4.5:1 ratio)"

    Force dark в Chrome — это accessibility-фича (для пользователей, которым нужен тёмный режим). Если в этом режиме CTA становятся нечитаемыми — это доступность ломается.

  • Реакция Ивана (2026-04-23):

    I'm trying to find solution for this problem, in research

    → не зарезолвил в тот же день. Признал проблему, ушёл искать решение.

  • Обоснованно? Да. Это валидный accessibility-кейс. Решение — обычно зафиксировать color-scheme: light для CTA или прописать явные цвета, которые не инвертируются.

  • Серьёзность: средняя. Не критический баг прода (force-dark — экспериментальная Chrome-фича), но касается доступности, которая в спеке прямо есть.

  • Что важно:

    • Это первое замечание в этом PR, на которое Иван не дал немедленного решения — нужно "research". Это может затянуть PR.
    • Тестирование в dark mode / force-dark — не базовый QA. Здесь претензию можно частично смягчить тем, что Иван сам Chrome flag включает редко. Но WCAG требует доступности.
    • Опять же — внутренний процесс: эта проверка должна быть в чеклисте перед отправкой PR на ревью клиенту, особенно когда спека прямо упоминает WCAG AA.

  • Вопрос Ивану: есть ли у тебя в чеклисте проверка WCAG-контраста? Используешь ли тулзы (Lighthouse, axe DevTools, contrast-ratio plugin) перед открытием PR? Решение нашёл?

Накопительный счёт по PR #846 за 22.04–23.04 (2 дня)

#ЗамечаниеСерьёзностьСтатус
1Vercel deploy botслужебный—
2"Info For" dropdown center-justified — не в спекеResolved
3Меню в 2 строки на 1024–1091px (регрессия)Resolved
4Text-only logo в expanded mobile не реализованResolved
5Dark mode → плохой контраст на CTA (WCAG)In research (не решено)

4 содержательных замечания за один день после открытия — все ложатся в "deviate from spec" / "internal validation". Это и есть header, про который Alexis пишет.

Comment #6 — Michael Forbes: дублирование CTA в mobile menu (Request Info / Visit / Apply показаны дважды) - Скриншоты:

  • — комментарий + код

  • — скрин mobile menu с зачёркнутыми дубликатами

  • — 3 коммита-исправления

  • Файл: components/header/header.js

  • Автор: Michael Forbes

  • Дата: 2026-04-23

  • Цитата:

    The menu items that match item.displayStyle === 'button' are already rendered via <MobileTopMenu items={utilityMenuItems} /> so they need to be removed from this <UtilityMenu> to avoid duplication.

  • Что видно на скрине: В раскрытом mobile menu сразу под основными секциями (Academics / Discover Stevens / Student Life / Research / Admission & Aid) идёт нижний блок-список: Corporate Relations, Athletics, Alumni, Give, Info For, Log In — и снова Request Info, Visit, Apply (зачёркнуты жёлтым крестом).

    То есть CTA-пункты появляются дважды:

    1. В верхней части mobile menu (как кнопки) — это корректное место по мокапу.
    2. Внизу в UtilityMenu — это дубликат.

  • Причина (по комментарию Michael): в коде <UtilityMenu> рендерит все пункты, включая те, у которых displayStyle === 'button'. Но эти "button" пункты уже отрендерены отдельно в <MobileTopMenu>. Нужно отфильтровать их в <UtilityMenu>.

  • Реакция Ивана: запушил 3 коммита 22d4df7, 8bb932a, 40041f9 (тот же 23.04). Тред когда-то был помечен resolved, потом видна кнопка Reopen — то есть Michael либо переоткрывал тред (исправление было неполным), либо это разные итерации обсуждения.

  • Обоснованно? Да. Это функциональный баг UX:

    • Пользователь видит одни и те же CTA дважды.
    • Лишний шум в навигации.
    • Если у CTA есть data-cta/data-location для аналитики (по спеке) — события могут дублироваться.

  • Связь со спекой:

    • Мокап v3 (Site Header v03 Sticky.png, секция "expanded mobile menu") показывает CTA только в верхней части — ниже их нет.
    • Спека про data-attributes: data-cta уникальный для каждой кнопки → дублирование делает аналитику невалидной.

  • Серьёзность: Средне-высокая. UX-баг видный, ловится открытием expanded mobile menu и беглым взглядом — это снова то, что должен ловить self-QA.

  • Связь с письмом Alexis:

    • "deviate from spec" — да, мокап не предполагает дублирования.
    • "internal validation pass" — да, ловится за 30 секунд.

  • Reopen важно: Если тред был переоткрыт Michael — это значит, что первое исправление Ивана не закрыло проблему полностью. То есть: исправил, но не до конца → пришлось доделывать. Это паттерн "сделал не вдумчиво, ревьюер вернул на доделку" — лишние раунды ревью.

Накопительный счёт по PR #846 (2 дня)

#ЗамечаниеСерьёзностьСтатус
1Vercel deploy botслужебный—
2"Info For" dropdown center-justifiedResolved
3Меню в 2 строки на 1024–1091pxResolved
4Text-only logo не реализован (mobile)Resolved
5Dark mode WCAG-контраст на CTAIn research
6CTA дублируются в mobile menu (UtilityMenu)Resolved (возможно с reopen)

5 содержательных замечаний за день после открытия. Все из категорий письма Alexis. Вместе с ранее замеченными PR #832 / #842 формируют системную картину.

Comment #7 — Michael Forbes: sticky-меню на 1024px+ работает неправильно (спека была двусмысленной) - Скриншот:

  • Автор: Michael Forbes

  • Дата: "7 days ago" — при сегодняшнем 2026-04-30 → ~2026-04-23

  • Цитата:

    The current implementation of the "sticky" menu (the one that stays at the top of the viewport when scrolling down the page) has a problem on large viewports (1024px+). It prevents users from seeing most menu items, and on hover it gives users some duplicate menu items.

    I was a little confused by the spec at first, but I asked the author of the spec, and he gave this clarification: The sticky menu for large screens (1024px+) should be exactly like the sticky menu for medium screens (768px-1023px). It needs to have a hamburger button, and it does not need to reveal the top gray bar on hover (like it does now).

    Аннотации на приложенном референс-скрине:

    • "Nav max width 840px / Menu items clickable"
    • "Bitter, 12px bold #FFFFFF"

  • Суть:

    • Sticky-поведение, которое реализовал Иван для desktop (1024px+), оказалось неправильным: меню скрывается + при наведении дублируются пункты.
    • Сам Michael признаёт, что спека была двусмысленной ("I was a little confused by the spec at first") — пришлось уточнять у автора спеки (вероятно Tom).
    • Уточнение: на 1024px+ sticky должен выглядеть как на medium screens (768–1023px) — с hamburger-кнопкой, без серой полосы на hover.

  • Это важный момент для нашей защиты: Это первый случай в этом PR, когда сама спека признаётся неоднозначной самим клиентом ("the author of the spec" гивает clarification после вопроса). То есть — отклонение реализации от ожидания клиента здесь частично объяснимо тем, что спека не была однозначной.

    • Если Иван интерпретировал её одним способом, а Stevens ожидал другого — это gap в спеке, не только в реализации.
    • Это можно использовать как смягчающее в разговоре с iX/Stevens: "ваш ревьюер сам признал двусмысленность; уточнение пришло уже в процессе ревью; реализация будет приведена к новой версии".
    • Но не злоупотреблять — на других пунктах (logo, dropdown, dark mode, регрессия) спека была чёткая.

  • Серьёзность: Средняя по существу, но высокая для нашего разбора — это важный пример, что и со стороны клиента есть пробелы.

Comment #8 — Michael Forbes: после фикса остаются 3 проблемы со sticky - Скриншот:

  • Автор: Michael Forbes

  • Дата: "6 days ago" → ~2026-04-24

  • Прикреплено: видео sticky-header.mov

  • Цитата:

    With the most recent update, I still see 3 issues with the "sticky" (scrolled down) header:

    1. Sometimes it has the primary nav ("ADMISSIONS" etc.) instead of the 3 buttons ("Request Info" etc.). In the video below, you can see it has the primary nav (bad) from 0:00 to 0:15 but then suddenly it has the 3 buttons (good) from 0:15 to 0:36.
    2. The gray bar no longer appears on hovering (good), but it does briefly appear when increasing the browser width across the 1024px breakpoint (bad). Around 0:24 and 0:27 and a few more times after that in the video.
    3. The hamburger button exists below the 1024px breakpoint but disappears above the 1024px breakpoint. It should exist for both.

    By taking the existing sticky nav used below 1024px, and having it also be used above 1024px, I think all three problems should get solved at the same time.

  • Реакция Ивана (~2026-04-27, "3 days ago"):

    @Michael Forbes I see what you mean. I'll fix that.

    Не зарезолвлено на момент скрина. Иван подтвердил, ушёл фиксить.

  • Что происходит:

    1. Race condition / state bug: sticky-header иногда показывает primary nav, иногда CTA-кнопки — переключение происходит неожиданно (на 0:15 в видео). Это баг логики (вероятно, какой-то resize/scroll observer срабатывает с задержкой или непоследовательно).
    2. Flash при ресайзе на breakpoint 1024px: серая полоса проскакивает (state не успевает синхронизироваться с layout).
    3. Hamburger исчезает выше 1024px — несоответствие уточнению из Comment #7 (должен быть на всех ширинах sticky).

  • Обоснованно? Да. Все три — реальные функциональные баги, видны в видео.

  • Важный сигнал: Иван уже сделал фикс после Comment #7 (sticky должен быть как medium-screen), но фикс неполный:

    • Возможно, Иван не сделал так, как буквально описал Michael ("take the existing sticky nav used below 1024px, and having it also be used above 1024px") — а пытался переделать сверху.
    • Это паттерн "не до конца понял, что просили".
    • Лишний раунд ревью.

  • Серьёзность: Высокая — это видимые баги в продовом preview (Michael записал видео).

Накопительный счёт по PR #846 (на 27.04, ~2026-04-30 момент скринов)

#ЗамечаниеСерьёзностьСтатус
1Vercel deploy botслужебный—
2"Info For" dropdown center-justifiedResolved
3Меню в 2 строки на 1024–1091pxResolved
4Text-only logo не реализован (mobile)Resolved
5Dark mode WCAG-контраст на CTAIn research
6CTA дублируются в mobile menu (UtilityMenu)Resolved
7Sticky на 1024+ — неправильное поведение (спека двусмысленная)Уточнено + фикс
8После фикса — 3 sticky-issues (race, flash, hamburger)"I'll fix that" — не зарезолвлено

8 содержательных замечаний за неделю (22.04 → 27.04). Один пункт в нашу пользу — спека была двусмысленной (Comment #7).

Comment #9 — Michael Forbes: 4 проблемы с search-фичей - Скриншоты:

  • — спека (мокап v3)

  • — что в feature branch

  • Автор: Michael Forbes

  • Дата: 2026-04-23

  • Цитата:

    4 issues with the search feature:

    • The text is specified as IBM Plex Sans (no serif), but this implementation has a serif font.
    • For the magnifying glass adjacent to the text input area, the spec shows gray-on-white, but this implementation has white-on-gray.
    • This implementation has a red border around the white text input area, but the spec does not.
    • The gray background is specified as being a continuous L shape, but it is discontinuous (two regions with a gap between them) in this implementation.

  • Суть: четыре отдельных визуальных расхождения в одном ограниченном куске UI (search-блок):

    1. Шрифт — IBM Plex Sans (sans-serif), а в реализации serif (видимо, унаследовался от родительского стиля).
    2. Цвет иконки лупы — должен быть тёмный на белом, реализован белый на сером.
    3. Лишний красный бордер вокруг input.
    4. Серый фон не L-образный (как должно быть), а разбит на 2 куска с дыркой.

  • Реакция Ивана (2026-04-23): "Resolved all" + встречный вопрос про alignment (см. Comment #10).

  • Обоснованно? Да, абсолютно. 4 параллельных отклонения от мокапа в одном небольшом блоке. Все ловятся 30-секундным сравнением "мокап ↔ preview" — снова internal validation.

  • Серьёзность: Высокая — четыре stylistic deviation в одном PR-блоке. Это по тяжести как отдельный пункт письма Alexis: "deviate considerably from spec".

  • Что важно для разговора:

    • Это классическая ошибка self-QA: реализуется логика, а финальная стилизация под мокап не сверяется построчно.
    • В iX-проектах с дизайн-сильным клиентом такого быть не должно — сравнение с мокапом обязательно.
    • Если бы у нас был figma access / overlay-tool / просто привычка положить мокап рядом с preview — пойманы за минуту.

Comment #10 — встречный вопрос Ивана + затянутая итерация alignment - Скриншоты:

  • — Иван "Resolved all, but..."

  • — Michael не понял, Иван переформулировал

  • — деталь alignment

  • — Michael консультируется с дизайнером Jay

  • — зелёная линия выравнивания

  • — Иван "Fixed"

  • Хронология (относительно ~2026-04-30):

ДатаКтоДействие
2026-04-23Иван"Resolved all, but Are you sure that equal line by icon and link will be better desition? Because when we open search modal, it became not equal in header."
~2026-04-24 (6 days ago)Michael"I don't understand the question, sorry. Can you explain it more?"
~2026-04-27 (3 days ago)Иван"I was asking whether the alignment will be correct when the search button is open?"
~2026-04-27 (3 days ago)Michael"The designer (Jay) has provided this clarification: it is meant for the magnifying glass and the X (not the gray background of the search modal) to be aligned with the edge of the yellow button area as it is in the mockup." + скрины с зелёной линией
~2026-04-28 (2 days ago)Иван"Fixed"

  • Суть: Пока Иван исправлял 4 search-issues (Comment #9), он встретил неоднозначность по выравниванию иконки лупы и крестика X относительно правого края жёлтых кнопок. Спросил у Michael. Michael не понял вопрос. Иван переформулировал на следующей итерации (через ~3 дня!). Michael пошёл к дизайнеру Jay за уточнением. Получили правило: лупа и X должны align с правым краем "Apply"-кнопки (не серого фона). Иван исправил.

  • Что важно:

    1. Иван заметил тонкость alignment — это в плюс (внимательность к мелочам).
    2. Но формулировка вопроса была плохая ("equal line by icon and link will be better desition") — Michael не понял, потеряли цикл.
    3. 3 дня между уточняющим вопросом и переформулированием — большой gap. Это потерянное время в ревью-цикле.
    4. Спека по этому конкретному alignment'у была неточна — потребовалось обращение к дизайнеру Jay. ✓ Ещё один пример пробела в спеке (как и Comment #7 про sticky).

  • Серьёзность: Средняя. По существу — это нормальный обмен с дизайнером. Но language barrier и медленный отклик растягивает PR.

  • Связь со встречей:

    • Защитный аргумент: "не вся вина наша — спека и здесь оказалась неточной, потребовался designer Jay для уточнения".
    • Уязвимый аргумент: "наш разработчик задаёт вопросы на ломаном английском, ревьюер тратит цикл на их расшифровку". Это процессный момент: возможно, нужно либо помощь с формулировкой коммитов/комментариев, либо менеджер/тимлид перечитывает перед отправкой.

Накопительный счёт по PR #846 на момент скринов (~2026-04-28)

#ЗамечаниеСерьёзностьСтатус
1Vercel deploy botслужебный—
2"Info For" dropdown center-justifiedResolved
3Меню в 2 строки на 1024–1091pxResolved
4Text-only logo не реализован (mobile)Resolved
5Dark mode WCAG-контраст на CTAIn research
6CTA дублируются в mobile menuResolved
7Sticky на 1024+ — спека двусмысленная (✓ в нашу пользу)Уточнено + фикс
8После фикса — 3 sticky-issues (race, flash, hamburger)"I'll fix that"
94 search-проблемы (font, иконка, бордер, L-shape)Resolved
10Alignment magnifier/X — спека неточная (✓ частично в нашу пользу), language barrierFixed

10 содержательных тредов за 6 дней (22.04 → 28.04). Из них 2 пункта частично в нашу пользу (#7 и #10 — спека требовала уточнения). 8 — наши промахи, в основном self-QA / сверка с мокапом.

Comment #11 — Michael Forbes: data-cta должен быть стабильным (item.sys.id вместо title-slug) ✓ частично в нашу пользу

  • Скриншоты:

    • — комментарий + код
    • — Иван показывает, что есть sys.id (Apply)
    • — Иван: "but it looks like that would be a confusing name for a button" (Request Info)
    • — Michael объясняет историю с "Giving" → "Give"

  • Файл: components/header/mobile-top-links.js

  • Автор: Michael Forbes

  • Дата: "6 days ago" (~24.04) → диалог продолжался до "21 hours ago" (~29.04)

  • Что сделал Иван (код):

    {items.map((item, index) => {
  if(!item.page && !item.externalLink) return null
  const dataCtaText = item.title.toLowerCase().replace(/\s+/g, '-')
  ...
  const analyticsProps = {
    'data-cta': dataCtaText,
    'data-location': 'header'

    То есть data-cta = slugified title: "Request Info" → "request-info", "Visit" → "visit", "Apply" → "apply".

  • Цитата Michael (24.04):

    The value for data-cta needs to be stable. For example if the menu item's title changes from "Visit" to "Visiting" but its purpose does not change, the data-cta must not change to ensure continuous analytics. Can we use something like item.entryId instead of item.title to achieve that stability?

  • Реакция Ивана (~27.04):

    For each button we have id [показывает в DevTools console: sys.id: "5wGGimgLFp7qrBr5lxb6eW" и т.п.]

    But it looks like that would be a confusing name for a button

    То есть Иван корректно нашёл sys.id, но возразил, что значения вида "5wGG..." человеку непонятны.

  • Реакция Michael (~29.04):

    The value for the data-cta attribute should be item.sys.id (for example "5wGG..." and "3Sjk..." even though those seem "confusing") assuming this is the only property that would be stable when the item.title happens to undergo small language changes. Changes like this do happen sometimes, for example about 16 months ago the item.title of this menu item changed from "Giving" to "Give" and the person requesting this data-cta attribute has specified that the data-cta value be stable across such changes.

    Post-deployment, if he determines that the analytics become too confusing because of this, we can find another solution.

  • Это очень показательный кейс — частично в нашу пользу:

    Реализация Ивана точно соответствует примеру в спеке:

    Acceptance Criteria (User Stories v2): "data-cta="request-info" — exact value to be confirmed" "data-cta="visit-campus" — exact value to be confirmed" "data-cta="apply" — exact value to be confirmed"

    То есть спека прямо приводит human-readable значения в качестве примеров. Иван следовал примерам.

    НО в той же спеке есть оговорка:

    "exact value to be confirmed" "A data attribute reference document or ticket is created and agreed by dev, analytics, and QA before implementation begins"

    Это значит:

    • Спека сама себя противоречит: показывает примеры (которые Иван взял за основу), но требует "agreement before implementation".
    • Согласования "before implementation" не было — никто не сказал Ивану, что значения должны быть стабильны через смену контента.
    • Аналитический контекст (история "Giving" → "Give" 16 месяцев назад) Иван не знал и знать не мог.

  • Серьёзность: ✓ 3-й пример пробела в спеке в этом PR (после Comment #7 sticky и Comment #10 alignment).

    Это сильный аргумент в нашу пользу:

    • Спека прямо требует pre-agreement, который не был сделан.
    • Иван пошёл по примеру, приведённому в спеке.
    • После уточнения — корректировка тривиальная (1 строка кода: использовать item.sys.id вместо slugified title).

  • Вопрос Ивану: хорошо ли понял требование "stable analytics value"? Знаешь ли разницу между display name и stable identifier для аналитики?

  • Вопрос Антону: была ли с нашей стороны проверка "spec говорит agreed before implementation — было ли agreement?". Если нет — это процессный gap (нужно было поднять до старта работы).

  • Связь с письмом Alexis:

    • "internal validation" — да, мы должны были увидеть в спеке "agreement before implementation" и поднять.
    • Но именно этот PR не нарушение, а пример того, что спека требует доработки на стороне клиента.

Накопительный счёт по PR #846 на ~2026-04-30

#ЗамечаниеСерьёзностьСтатусSpec gap?
1Vercel deploy botслужебный——
2"Info For" dropdown center-justifiedResolvedнет
3Меню в 2 строки на 1024–1091pxResolvedнет
4Text-only logo не реализован (mobile)Resolvedнет
5Dark mode WCAG-контрастIn researchнет
6CTA дублируются в mobile menuResolvedнет
7Sticky на 1024+ — двусмысленная спекаУточнено + фикс✓ да
8После фикса — 3 sticky-issues (race, flash, hamburger)"I'll fix"нет
94 search-проблемы (font, иконка, бордер, L-shape)Resolvedнет
10Alignment magnifier/X — нужно уточнение JayFixed✓ частично
11data-cta стабильность (sys.id vs title-slug)TBD (диалог)✓ да

11 содержательных замечаний в PR #846 за период 22.04 → 29.04 (8 дней).

Распределение:

  • 3 пункта (#7, #10, #11) — пробелы в спеке на стороне клиента → защитные аргументы.
  • 8 пунктов (#2, #3, #4, #5, #6, #8, #9 + регрессия) — наши self-QA / спек-согласование промахи.
<!-- блоки комментариев добавлять сюда -->