@@ · frontend review @@

How to review frontend pull requests beyond looks fine

Othman Shareef · August 13, 2026 · 6 min read · The Craft of Code Review

Frontend PRs collect the emptiest approvals in the codebase. A reviewer reads the JSX top to bottom, the markup looks plausible, the styles look plausible, and “looks fine” ships. The problem is that frontend correctness barely lives in the diff: it lives in what happens between renders, on slow networks, at odd viewport widths, and under a keyboard instead of a mouse. To review frontend pull requests well, review behavior. The diff is just the map, and the map is not the territory.

Review frontend pull requests in states, not screens

The single highest-value habit: for every piece of UI in the diff, walk the four states explicitly.

  • Loading: what renders while data is in flight? A layout that jumps when content arrives is a bug you can spot in the diff by the absence of a skeleton or reserved space.
  • Error: what does the user see when the request fails? “Nothing, the component just does not render” is the most common answer and the wrong one.
  • Empty: zero items is not an error and not a loading state. If the diff maps over a list with no empty branch, ask.
  • Success: the one state the author tested. Spend your remaining attention on the other three.

Race conditions hide between the states

State handling bugs rarely show up as wrong code; they show up as code that assumes events arrive in order. The classics are easy to spot once you name them. A typeahead fires a request per keystroke, and the response for “re” lands after the response for “react”, overwriting fresh results with stale ones: look for request cancellation or a staleness check. A submit button without a disabled-while-pending state will double-fire on a slow connection. An async callback updates state on a component that unmounted two navigations ago. When the diff adds any await between a user event and a state update, ask the ordering question: what happens if a second event fires before the first one resolves? If the answer requires optimism, it requires a fix.

Accessibility basics that take two minutes to check

You do not need an audit to catch the accessibility failures that matter most; you need a short list applied every time. Inputs have labels, not just placeholders. Interactive things are buttons or links, not a div with an onClick, because the div gets no keyboard or screen-reader behavior for free. Modals move focus in when they open and restore it when they close. Images that carry meaning have alt text; decorative ones have an empty alt. Text contrast survives the muted-gray-on-white aesthetic the design system keeps drifting toward. None of these require expertise to review, and catching them in the PR costs a comment; catching them after ship costs a ticket, a sprint, and an apology.

When to pull the branch and click around

Reading a diff answers “is this code reasonable”; only running it answers “does this work”. The threshold for pulling the branch should be low and explicit: any new user flow, anything animated, anything with focus or keyboard behavior, any layout change that must survive mobile widths. Once it is running, resize the window through the breakpoints, throttle the network in devtools and watch the loading states you just reviewed on paper, and tab through the new UI without touching the mouse. We have written before about reviewing locally versus in the browser; frontend changes are the strongest case for local, because the browser tab showing you the diff cannot show you the product. This is also where review tooling earns its keep: our own reviewer, Pyor, exists partly because switching between a fast diff surface and a running branch should not cost you your place in the review.

Bundle size and the dependency question

Every frontend PR that touches package.json is two reviews: the code and the dependency. The dependency review asks whether the platform already does this (surprisingly often: dates, formatting, clipboard, dialogs), whether the UI library already ships the component being rebuilt, what the new package weighs and whether it tree-shakes, and whether it is maintained by more than one tired person. Lockfile diffs measured in thousands of lines for a convenience helper deserve a conversation, not an approval. The reviewer is the last person positioned to ask, because after merge the dependency is load-bearing and the question is moot.

Screenshots and recordings are a review accelerant

The cheapest improvement to frontend review does not happen in review at all: it happens in the PR description. Before-and-after screenshots for visual changes, a short recording for interactions, viewport captures for responsive work. This is author self-review doing double duty: producing the capture forces the author through their own states, and the reviewer starts from observed behavior instead of reconstructing it from JSX. SmartBear’s review research found reviewers process only a few hundred lines per hour well; screenshots let you spend those lines on logic instead of mentally rendering markup. Teams that make captures a checklist item for UI changes review faster and argue less, because everyone is looking at the same pixels.

Frequently asked questions

What states should every frontend change handle?

At minimum four: loading, error, empty, and success. The diff usually shows success and nothing else. Reviewers should ask what renders while data is in flight, what the user sees when the request fails, and what an empty result looks like. Most frontend bugs that reach users are not broken success states; they are missing or wrong handling of the other three.

When should I run a frontend branch locally instead of reading the diff?

Whenever the change is interactive or visual: new flows, drag and drop, animations, layout changes, anything with focus or keyboard behavior. Reading JSX tells you what the tree renders; it cannot tell you how it feels, whether focus lands sensibly, or what happens on a slow connection. Ten minutes of clicking with throttled network catches what an hour of reading misses.

How should reviewers handle new frontend dependencies?

Treat every new dependency as a standing cost, not a one-time import. Ask three questions: does the platform or an already-installed library do this, what does it add to the bundle, and who maintains it? A date-formatting one-liner does not justify a library, and a component your UI kit already ships should never be rebuilt from a new package.

← All posts