DEV Community

Abidit Shrestha
Abidit Shrestha

Posted on

The bug I "fixed" that taught me more from being closed than merged

A few weeks ago I opened a PR on a large open-source design system to fix a contrast issue I'd spotted in dark mode — a text/background color pairing that failed WCAG AA. I patched it, added regression tests, and got an initial approval. Then the maintainer reversed course, and eventually the PR was closed outright. Here's what happened, and why I think it was the right call.

Before and after contrast comparison — 4.27:1 failing WCAG AA versus 4.37 to 10.57:1 passing WCAG AA

The bug

The component in question used light-dark() set inline to switch colors between light and dark mode. In dark mode, the contrast ratio between the text and background token pairing was below the 4.5:1 threshold for WCAG AA. My fix bumped it into a 4.37–10.57:1 range depending on state — a clear improvement, and it got an initial approval.

Where it went sideways

Before merging, the reviewer went back and actually traced the token pairing across the rest of the system — and found the same underlying tokens failed contrast in several light-mode themes too, not just dark mode. A second component using the identical token pairing had the same latent bug.

The problem: my fix lived inside one component (Avatar.tsx). A light-dark() call set at the component level can only ever resolve one branch per color scheme. It fixed the specific failure I'd noticed, but it structurally couldn't reach the other failures, because they lived in other components consuming the same broken token pair. Patching one consumer left every other consumer of that token exactly as broken as before.

Diagram showing one shared design token flowing into three components — Avatar.tsx, AvatarGroupOverflow.tsx, and Kbd.tsx — each inheriting the same contrast bug

The actual fix — and why it wasn't mine to make

The reviewer's conclusion was that this needed to be fixed at the design token definition itself, not in any individual component. That's the only place a fix propagates to every consumer and every theme at once, instead of playing whack-a-mole component by component.

A few days later, the PR was formally closed with a note that color-token and palette changes are now being coordinated as owner-led accessibility work, rather than through parallel, uncoordinated implementation PRs from individual contributors — and pointed toward a tracking issue for the broader initiative.

They also linked a separate, already-merged PR that added a lint rule forbidding light-dark() in component source going forward — codifying the exact anti-pattern my PR had (accidentally) demonstrated.

What I took away from this

  1. A locally correct fix can still be the wrong fix. My patch genuinely improved contrast for the case I tested. It was still wrong, because it solved the instance instead of the root cause, and would have left the system with two different fixes to the same underlying problem living in two different places.

  2. Look for the shape of the bug, not just the instance. The real signal was asking "does anything else in this codebase depend on the same token pairing?" — which is a question about the system, not the component. That question is what turned a one-line fix into a systemic finding.

  3. Getting your PR closed isn't the same as being wrong. The maintainer was explicit that the finding was correct and valuable — the fix itself was closed on architectural grounds, not because it was broken. Open source review, done well, is about protecting the health of the whole system even when it costs an individual contribution its merge.

  4. A closed PR can still be a good outcome. The findings led to a lint rule preventing the anti-pattern going forward, and to color-token ownership being reorganized into a single coordinated effort instead of scattered fixes. That's a better long-term result than my one component getting a green checkmark.

If you're contributing to a large open-source codebase for the first time: it's worth getting comfortable with the idea that "correct and closed" is a completely normal outcome, and often a sign the review process is doing exactly what it's supposed to.

Top comments (0)