Repository navigation
ROU-13041: Document that the Storybook Theme toggle is the only gate on a classic-theme port - #1285
Merged
Merged
Conversation
Record the spike's decision: the Cucumber functional suite asserts behaviour, Chromatic asserts appearance. Adds the ADR, its row in the ADR Log, and the Markdown source of the Confluence summary published for the wider team. - D1 makes the ownership assignment explicitly; research found no prior ticket, ADR or Confluence decision that ever transferred it. - D1a-D1d give the per-assertion conversion rules: the operation is mostly substitution, not deletion, and no bulk edit keyed on cluster label, expected value or file name is safe. - D1e records the one gap the split creates that no gate in either repository would detect, with ROU-13032 as the assessment that already measured it. - D2 (classic-theme visual coverage) is left deliberately open. - D4 closes DatePicker with zero candidate defects; D4a carries the Submenu underline as the one unresolved candidate, unverified. - D5 records the PR gate's unresolved template reference as unverified. - D6 evaluates the Playwright rewrite without adopting it. - D7 records that --legacy filters nothing, so no failure count from the suite is interpretable until it is fixed. No test code, no pipeline configuration and no runtime change under this ticket.
The follow-up work was described but scattered across D2, D4a, the next-steps prose and the notify list, with no owner and no at-a-glance view. Three actions never reached the list at all: D2's two hardening moves, and the open decisions (Q1, Q14, Q15). Adds "Action items at a glance" at the top of Recommended next steps — 16 rows (A0a, A0b, A1-A14) with type, where it lands, owner and rough size — and tags the three standing practices G1-G3 so the sequence below can reference them. No content changed; this consolidates what the ADR already said.
The page carried only the three asks (Q12, Q1, Q9/D7); the rest of the follow-up work existed only in the ADR, which the wider team does not read. Replaces "What we need, and from whom" with a full "Action items" table — 16 rows (A0a, A0b, A1-A14) with owner and rough size, the three rows needing a human answer called out, and the three standing practices summarised. Republished to page 6871941170. The page is now ~1,240 words, over the one-screen proxy, which is the deliberate trade for making the action list visible where the team will see it.
The summary page lives in Confluence (page 6871941170); keeping a second copy in docs-internal/adr/ added a file to review that is not the published artifact and would drift from it. Removes ADR-0011-confluence-summary.md and the ADR Links entry that pointed at it. The ADR still links the published page; the page still names the ADR, so the two cross-link. This PR is now the ADR plus its index row. Consequence, recorded deliberately: AC8c (commit the page source alongside the spec) is no longer met in any form. Confluence is the single home for that content, and republishing means editing the page rather than re-running `koda tool confluence update` from a tracked file.
Two changes to ADR-0011. Removes the "Deliverable shape" option group and its outcome. It was a decision about this spike's own paperwork, not an architecture decision, so it does not belong in a durable record. The Playwright group is relabelled C, the outcome line becomes "A -> Option 4. B -> open (Q1). C -> Option 2.", and the "ADR alone, without the Confluence page" rejected alternative goes with it. The three alternatives AC5 requires are untouched. Removes every hyperlink. This repository is public, so Jira and Confluence URLs both leak internal addresses and are unreachable for an outside reader; they are now mentions by ticket ID and page name. The sibling-ADR links are mentions too, reading "all internal links" literally. NFR4 still holds -- ADR-0004, ADR-0008 and ADR-0009 are each named with why they matter. Note this leaves FR1 partially unmet by intent: it asked for D3 to be recorded.
Two findings landed on 2026-10-06 and both answers were the unwelcome one. D5 / Q9 — the functional PR gate does not compile, confirmed. The PR carrying this ADR triggered a run that failed at template resolution: the referenced pipelines/templates/build-and-execute-functional-tests-template.yml does not exist in outsystems-ui-tests, and the error names the tests repo's current default-branch commit, which rules out the two alternative explanations the section had left open (a different YAML, or a pinned branch). The gate has been dark since 2026-08-07, so the 274 failures were never a release blocker. The "unblock the release" framing is retired explicitly. D4a / Q12 — the Submenu hover/active underline is a real theme regression, not an intended redesign. It is the only defect three full passes found, and its message was indistinguishable from the 111 cosmetic colour failures around it, so every bulk-edit approach would have turned it into a passing test. Action items A0b and A1 change from investigations to fixes; Status, C4, D4a, D5 and the step sequence are updated to match. The Confluence page (6871941170) was republished with the same changes plus a short "Resolved" block.
The research recommended adding classic-theme to the visual-surface path list at chromatic.yaml:76, on the grounds that a classic-only PR "silently disarms the capture guard". That reasoning does not hold. The path list means "things that can move a snapshot" -- the workflow says so in its own comment. A change under classic-theme/ cannot move a snapshot, because no story loads that CSS. The guard is therefore not disarmed, it is correctly inapplicable, and adding the path would make it assert something vacuous: Chromatic photographs ~113 new-theme stories on every build, so the check would pass unconditionally. The real cost is comprehension. Listing classic-theme there asserts it is part of the visual surface Chromatic covers, and a future reader would reasonably conclude the stories are verified in both themes. They are not -- that is Option B, which D2 leaves open. D2 now records one hardening move instead of two, with the withdrawal and its reasoning kept so the recommendation is not re-derived later. The action table drops the row and renumbers A9-A14 to A8-A13; the ADR-0008 link entry and G2's cross-reference follow. If Option B is ever adopted, the path belongs in that list as part of that work.
…gate Implements A8. The skill already told you to flip the Storybook Theme toolbar and check the ported rule (section 8); what it did not say is that nothing else is watching. Everything else in this repo is covered by CI, so the reasonable assumption is that a port is too. It is not: the functional tests derive their tags from src/scripts/** and src/scss/04-patterns so a classic-only change runs none; Chromatic photographs the new-theme bundle and no story loads classic-theme CSS; build compiles src/ to dist/; lint covers .ts; and there is no SCSS behind classic-theme/ to validate. Skip the visual check and a broken port ships with nothing to report it. Also records two limits of the toggle so it is not over-trusted: it links only the ODC bundle, so it validates half of a port; and it is new-theme runtime plus old-theme CSS, so it is a CSS-regression check rather than "the classic theme under test". Decision recorded in ADR-0011 section D2.
Q15 ("who owns the O11 a11y visual impact") was carried into the ADR as an open
question, but the specification had already closed it on 2026-10-06 as a split:
who fixes the over-specific platform selectors belongs to ROU-13032 and the
Frontend Runtime team, not to this spike, and the notification-channel half was
withdrawn as a mistaken premise. Listing it as open contradicted the source.
D1e now leaves one question rather than two -- Q14, how a future change of this
kind gets detected -- and says plainly that ownership of the current instance
sits with ROU-13032. D1e-1 keeps the governance content and the warning that it
can evaporate when that ticket closes; it just no longer pretends to be a
question this spike opened. The action table drops the row and ends at A12.
This makes FR8b partially unmet by intent: it asked for Q14 and Q15 to be
separated. There is nothing to separate when one of them is closed.
…on (Q16) D6 evaluated the rewrite and concluded it does not change D1. That still holds -- the behaviour/appearance split is the same decision whichever runner executes it -- but "it does not change D1" was being used to carry "there is nothing to decide", and those are not the same claim. What makes it live, and what changed since D6 was written: D5 is now confirmed. This ADR assigns behaviour ownership to the Cucumber suite, and that suite's PR gate has not compiled since 2026-08-07. The strongest argument for the incumbent was that it is the thing that actually runs, and it is not. Three directions compete and nobody has chosen: stay as-is, converge on ROU-12962's shared WDIO + Cucumber layer, or move to the rewrite. Doing nothing picks the first by default rather than on the evidence. The case against adoption is kept intact and unsoftened -- the rewrite widens the theme coupling from one file to 28, has no ADR, no CODEOWNERS, no PRs and one author, needs a new cross-repo template contract, and would strand ROU-12962. Four entry conditions are stated so the evaluation starts from something. Also writes out the four determinism fixes in full, with their commits, so the ADR is self-contained and the hand-off to ROU-13059 carries content rather than a promise -- and notes that only two of the four map onto the PR gate's browser matrix, which the earlier wording over-claimed. Adds a Considered Options entry for ruling the rewrite out, so the chosen option is visible as a choice; updates Status, the outcome line, and the action table (A13).
…dict Q16 is an open question, so listing the rewrite's gaps as reasons it is "worse" prejudged it. Replaces that with a seven-row table of what would have to change for it to be adoptable, each tied to the evidence and sized. The seven: apply D1 to it rather than build a palette module; measure parity by assertion rather than by scenario title; add a PR-triggered pipeline and a report format the cross-repo contract can consume; add CODEOWNERS, PRs and a sanctioning ticket; delete or re-sync the stale native/ fork; use or remove the unused page-object layer; and reconcile with ROU-12962. One substantive correction falls out of item 1. "The coupling widens from one file to 28" is true of the rewrite as it stands, but it is an artefact of the pre-D1 world: once appearance assertions leave the functional suite, most of those 50 theme-derived literals have no reason to exist in either codebase. That makes the coupling a reason to sequence D1 before adoption, not evidence the rewrite is structurally wrong. The D6 prose and the Considered Options entry are reworded to match. Item 2 is new and sharper than anything in the previous framing: scenario parity was measured by title, and the rewrite never ported the colour assertion for the current flatpickr widget. 1,376 of 1,414 matched says nothing about what each scenario checks.
Three corrections. 1. D4a is cut from ~50 lines to four. The verdict is in, so the evidence that produced it -- the computed-style table, the failure message, the related text-decoration failure, the argument about why it mattered -- is no longer what a reader needs. What they need is that the defect is real and has to be fixed. The Submenu still appears in D1b and D1c, where it serves a different purpose: it is the worked example for reading the found value rather than the expected one, which AC2b and AC2c require. 2. Q1, Q9, Q12, Q14 and Q16 were cited 24 times and defined nowhere. They are the working specification's numbering, and that document does not ship, so every citation was a dangling reference for anyone outside this spike. Adds an "Open questions" section defining all five with their status and where each is decided, and noting the two the specification carried that are deliberately absent. 3. The decision numbering skipped D3 with no explanation. D3 was the deliverable-shape decision, removed earlier because it was about this spike's paperwork rather than the architecture. The gap is now explained in Decision Outcome, and kept rather than closed by renumbering -- renumbering would silently break every D-number already in use against the specification.
Restructures the ADR for directness without dropping evidence. Every figure, citation, commit SHA, ticket reference and table from the previous revision is retained and was verified present after the edit. - Status becomes a per-decision table; adds a Summary section so the document has an entry point. - Considered Options A/B/C become tables; B and C now defer to D2 and D6 rather than restating their analysis. - Removes the numbered next-steps sequence, which duplicated the action table row for row; its unique content (the hygiene list and the TypeScript/lint posture) moves under A6, and the action table gains an 'After' column for the sequencing it carried. - Removes the 'Tickets that must be notified' section; its reasoning folds into the A9 and A10 rows. - Merges D5 with its 'Confirmed' subsection now that Q9 is closed. - Trims defensive meta-commentary and restates facts once in their owning section, referencing by label elsewhere. 1082 -> 882 lines, 11958 -> 9864 words, 47 -> 70 bullet lines.
The ROU-13032 assessment was a summary of another ticket's spike taking a subsection of its own. It becomes a closing paragraph of D1e, keeping the load-bearing points: the three findings that bound the risk, the visual-only conclusion that supports the ownership split, the Playwright screenshot-diff method as the cheapest known answer to Q14, the governance question being ROU-13032's to close, and the three manual findings behind Q14's time-based cadence. Dropped with the section: the per-component measurement list (Button, Input, Popover, Pagination, ActionSheet, Popup, validation message) and the two sides of the governance disagreement. Both remain in ROU-13032 itself and in the working documents. Also repoints the Open questions cross-reference from D1e-1 to D1e. 882 -> 857 lines.
The Links section said the specification and the three-run report are attached to the Confluence page. They are not attached anywhere - they live in a gitignored specs/ directory in the spike author's checkout and are not published. Names the files and states plainly that the ADR is self-contained, which is why nothing depends on them.
Keeps the 'attached to the Confluence page' claim and makes it checkable by naming the files: spec.md, rerun-report.md, and the two research documents behind Q1 and Q16.
BenOsodrac
previously approved these changes
Oct 6, 2026
|
🖼️ Storybook: Chromatic UI tests result: ✅ Passed
|
OS-giulianasilva
changed the title
OS-susmitabhowmik
approved these changes
Oct 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of ROU-13041. Documentation only: no test code, no pipeline configuration, no runtime change.
What this changes
.claude/skills/osui-classic-theme/SKILL.mdWhy
Nothing automated covers a port to the classic theme:
src/scripts/**andsrc/scss/04-patterns, so a change that only touchesclassic-theme/runs none of them.classic-theme/CSS.npm run build,npm run lintand a compiler do not reachclassic-theme/.The skill now says so plainly, so whoever does a port does not have to infer that nothing else is watching. This is action A8 of the ROU-13041 spike. It needs no separate ticket.
The two limits of the toggle are also recorded: it loads the ODC bundle only (so the O11 edit has to be checked by reading the diff), and it is a hybrid of new-theme JavaScript with old-theme CSS rather than the classic theme itself.
Decision record
The decision is recorded in ADR-0010,
docs/adr/ADR-0010-Split-Functional-Test-And-Chromatic-Ownership.mdinOutSystems/outsystems-ui-tests(PR #651), section D2. The team accepted the loss of classic-theme visual coverage on 2026-10-16.The team-facing summary is the Confluence page Functional tests and the new theme — testing strategy (space
RDMBLVS).