Repository navigation
feat(browser): show every page load, link clicks included, as a line along the address bar on desktop and mobile - #31597
Merged
Merged
Conversation
AmoabaKelvin
marked this pull request as ready for review
October 21, 2026 18:16
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description is detailed and covers the user impact, implementation, rationale, visual proof, testing, compatibility, and performance. However, the required Linked Issue section states “None” inste… | Add the actual linked issue using the required “Fixes #” format. If the author is a maintainer and no issue is required, state that explicitly according to the repository policy and obtain reviewer confirmation. |
| Docstring Coverage | |
Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 30 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: showing page-load progress along the address bar on desktop and mobile, including link clicks. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Description check
-
Explanation
The description is detailed and covers the user impact, implementation, rationale, visual proof, testing, compatibility, and performance. However, the required Linked Issue section states “None” instead of providing an actual issue link.
Resolution
Add the actual linked issue using the required “Fixes #<issue-number>” format. If the author is a maintainer and no issue is required, state that explicitly according to the repository policy and obtain reviewer confirmation.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
3eb5c6cf-ec10-43fe-bb8a-773c7e29b1a7
📒 Files selected for processing: 36
config/scripts/happy-dom-animation-cancel.tsconfig/scripts/vitest-dom-setup.tsconfig/tsconfig.node.jsonsrc/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.dead-guest.test.tsxsrc/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/BrowserAddressBar.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-load-progress.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-navigation-control-row.test.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-navigation-control-row.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-header.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-pane.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-page-toolbar.tsxsrc/renderer/src/components/browser-pane/assemble-chrome/browser-reload-control.tsxsrc/renderer/src/components/browser-pane/browser-client-page-guest-metadata.tssrc/renderer/src/components/browser-pane/host-guest/attach-browser-page-webview.tssrc/renderer/src/components/browser-pane/host-guest/bind-browser-page-webview-listeners.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-annotation-document.test.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-favicon-navigation.test.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-mount-admission.test.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-webview-loading-handlers.tssrc/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.tssrc/renderer/src/components/browser-pane/host-guest/use-browser-page-webview-lifecycle.tssrc/renderer/src/components/browser-pane/navigate/chromium-error-page-poll-visibility.test.tssrc/renderer/src/components/browser-pane/navigate/navigate-browser-page-notebook-link.test.tssrc/renderer/src/components/browser-pane/navigate/navigate-browser-page-url.tssrc/renderer/src/components/browser-pane/navigate/use-browser-page-navigation-downloads.file-drop.test.tsxsrc/renderer/src/components/browser-pane/navigate/use-browser-page-navigation-downloads.tssrc/renderer/src/components/browser-pane/navigate/use-browser-page-reload-actions.test.tsxsrc/renderer/src/components/browser-pane/navigate/use-browser-page-reload-actions.tssrc/renderer/src/components/browser-pane/navigate/use-browser-page-webview-url-sync.tssrc/renderer/src/components/browser-pane/restored-client-hosted-recovery-window.test.tsxsrc/renderer/src/components/browser-pane/stream-remote/remote-browser-page-pane.tsxsrc/renderer/src/components/browser-pane/stream-remote/remote-browser-page-toolbar.tsxsrc/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.tsxsrc/renderer/src/components/browser-pane/workspace-doc/doc-preview-toolbar.tsxtests/e2e/browser-reload-feedback.spec.ts
💤 Files with no reviewable changes: 15
- src/renderer/src/components/browser-pane/navigate/navigate-browser-page-notebook-link.test.ts
- src/renderer/src/components/browser-pane/host-guest/browser-page-mount-admission.test.ts
- src/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.tsx
- src/renderer/src/components/browser-pane/host-guest/attach-browser-page-webview.ts
- src/renderer/src/components/browser-pane/assemble-chrome/browser-navigation-control-row.test.tsx
- src/renderer/src/components/browser-pane/navigate/chromium-error-page-poll-visibility.test.ts
- src/renderer/src/components/browser-pane/host-guest/browser-page-annotation-document.test.ts
- src/renderer/src/components/browser-pane/workspace-doc/doc-preview-toolbar.tsx
- src/renderer/src/components/browser-pane/assemble-chrome/browser-page-pane.tsx
- src/renderer/src/components/browser-pane/navigate/use-browser-page-navigation-downloads.file-drop.test.tsx
- src/renderer/src/components/browser-pane/navigate/use-browser-page-navigation-downloads.ts
- src/renderer/src/components/browser-pane/navigate/use-browser-page-webview-url-sync.ts
- src/renderer/src/components/browser-pane/host-guest/use-browser-page-webview-lifecycle.ts
- src/renderer/src/components/browser-pane/navigate/navigate-browser-page-url.ts
- src/renderer/src/components/browser-pane/host-guest/browser-page-webview-loading-handlers.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| ) | ||
| await expect.poll(() => releaseHeld !== undefined, { timeout: 10_000 }).toBe(true) | ||
| await expect.poll(getLoading, { timeout: 10_000 }).toBe(true) | ||
| await expect(pane.locator('[data-browser-load-progress="loading"]')).toHaveCount(1) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Verify that the loading line becomes visible.
data-browser-load-progress="loading" is set before the 200 ms display delay. This assertion can pass while the line remains transparent, and Line 158 can release the response before the line appears. Wait for the line’s computed opacity to become nonzero before release. Apply the same check to the held-reload assertion at Line 95. As per coding guidelines, “assert user-visible behavior through the DOM.”
Source: Coding guidelines
AmoabaKelvin
marked this pull request as draft
October 21, 2026 18:44
Link clicks, form posts, redirects and history moves inside a page never set loading, because only navigations Orca started armed did-start-loading. Main-frame did-start-navigation now owns the start, so iframe loads and same-document changes stay out.
…reduced motion mid-load
…unts A long-running animation kept ticking the main thread for hidden or closed tabs. happy-dom now marks a cancelled animation's finished promise handled, as the spec and Chromium do.
…unread controls.loading
… and follow reduced motion mid-load
AmoabaKelvin
force-pushed
the
browser-loading-indicator
branch
from
October 21, 2026 19:17
5ce312e to
ef7873d
Compare
AmoabaKelvin
changed the title
AmoabaKelvin
marked this pull request as ready for review
October 21, 2026 23:23
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
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.
ELI5
When a page in Orca's browser is loading, a thin blue line now grows along the bottom of the address bar and fades out when the page is done. It also works for links you click inside a page, which before showed no sign of loading at all. The phone shows the same line when it's looking at a desktop browser tab.
What Changed
Before, the only cue was the reload button turning into a small spinner. Worse,
BrowserPage.loadingonly turned on for loads Orca started itself (typing a URL, reload). A link click, form post, JS redirect or in-page back/forward left it off for the whole server wait. On main, a link to a page that takes 4s shows nothing for 4s.Now:
loadingstarts on a main-frame, cross-documentdid-start-navigation. That replaces the gateddid-start-loading, so thetrackNextLoadingEventRefgate and its plumbing are gone. Iframe loads and SPApushState/hash changes don't count, which is the same rule Chromium uses for its own loading UI. Client-hosted pages use the same rule through one shared check (isPageDocumentNavigationStart).BrowserLoadProgresssits inside the address bar, on its bottom border, clipped to the rounded corners. It is 2px of--callout-note(GitHub's accent blue,#0969dalight /#4493f8dark). It waits 200ms before showing, eases toward 90% over 30s, then fills and fades when the load ends. A failed load fades where it stopped. It runs on Web Animations overtransformandopacityonly, and nothing runs once a load ends or the tab closes.MobileBrowserLoadProgress), with the same delay, curve, finish and fade-in-place on failure. It readsloadingandloadError, which the desktop already sent in the session tab and the phone ignored, so there's no wire change. It uses React Native'sAnimatedwith the native driver (transform and opacity), mobile's existingaccentBluetoken, and the existing reduced-motion hook.finishedpromise as unhandled, unlike the spec and Chromium. That's patched in the DOM test setup next to the other happy-dom patches.Why
The line matters less than the rule behind it. A bar driven by the old
loadingwould have stayed blank for most real browsing.Design
Chosen: one boolean that the code already had (
loading), fixed at its owner, plus a time-based line. Electron gives no load percentage, so the line eases on its own and the real stop finishes it. That's how Chrome Android, t3code and NProgress do it too.Ruled out:
loading. Link clicks, redirects and history moves would show nothing.did-start-loadingcount. Iframe polls would run the bar every few seconds.blue-500(the tab spinner's blue). The design-system gate rejects raw palette colours.--callout-notealready serves as the app's accent blue (chat unread, Jira) and is tuned per theme.Timings: 200ms show delay (STYLEGUIDE's deferred-feedback rule; Turbo uses 500ms, NProgress none), then 15% at 0.3s, 45% at 2s, 70% at 6s, 85% at 15s, 90% at 30s, then hold. The finish takes 450ms. With reduced motion it is a still full-width line at half opacity.
Open questions for review:
ERR_ABORTEDis swallowed upstream on purpose. I think filling is fine since you pressed Stop yourself.blue-500, which is close but not the same.Not covered yet: browser tabs owned by a headless
orca serve(the phone getsloading: falsefor those) and remote-streamed pages, whoseloadingonly covers Orca's own RPCs. Both need the host to report page loads, which is a wire change. That's a separate follow-up.Linked Issue
None. I found no issue or PR for this.
Visual Proof
The line mid-load at 0.6s, 2.5s and 7s, then reduced motion. Light, then dark.
Phone viewing a desktop tab (Android emulator, over-the-air page built from each branch). Sequence: typed slow URL, link click inside the page, fast page, then a server that drops the connection. Before on top, after below.
mobile-pair.mp4
The same component in a native (non-page) build, driven by a scripted harness: slow load, fast load, failed load.
native-harness.mp4
Desktop. Each video has before (main) on top and after (this branch) below. Recorded from a hidden dev build against a local server that holds responses.
Link click inside the page, server holds 4s. Before: nothing at all for 4s. After: the line runs for the whole wait. This is the main behaviour fix.
Before on top, after below.
Light
link-click-light.mp4
Dark
link-click-dark.mp4
Typed URL, server holds 4s. Before: small spinner on the reload button. After: the line, plus an X for Stop.
Before on top, after below.
Light
slow-load-light.mp4
Dark
slow-load-dark.mp4
Slow headers, streamed body, slow image. The line keeps going until the image finishes, then fills and fades.
Before on top, after below.
Light
staged-load-light.mp4
Dark
staged-load-dark.mp4
Fast pages back to back. No line at all. Loads under 200ms show nothing.
Before on top, after below.
Light
fast-load-light.mp4
Dark
fast-load-dark.mp4
New navigation while one is in flight. The line carries on. No jump back to zero.
Before on top, after below.
Light
renavigate-light.mp4
Dark
renavigate-dark.mp4
Stop mid-load. The line fills and fades, like Chrome Android does on stop.
Before on top, after below.
Light
stop-light.mp4
Dark
stop-dark.mp4
Unreachable host, then a server that drops the connection. The instant failure shows nothing. The slow one fades out where it stopped instead of filling up.
Before on top, after below.
Light
error-light.mp4
Dark
error-dark.mp4
Reload a 1.5s page. Same line as any other load.
Before on top, after below.
Light
reload-light.mp4
Dark
reload-dark.mp4
SPA route change (fetch, then pushState). No line, same as Chrome and Safari.
Before on top, after below.
Light
spa-light.mp4
Dark
spa-dark.mp4
Iframe that reloads every 3s. No line and no tab spinner. Iframe loads are not page loads.
Before on top, after below.
Light
iframe-light.mp4
Dark
iframe-dark.mp4
Tab A loading, switch to idle tab B and back. B shows no line while A keeps its tab spinner. Back on A the line is further along, not reset.
Before on top, after below.
Light
tab-switch-light.mp4
Dark
tab-switch-dark.mp4
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
New e2e in
tests/e2e/browser-reload-feedback.spec.ts: a link click inside the page whose response the server holds. It setsloadingand shows the line, and fails on main (loadingstays false). The reload case now checks the line instead ofanimate-spin.Unit: iframe loads and same-document navigations don't report loading (
browser-page-favicon-navigation.test.ts). Fixtures updated for the removed gate.Live in a hidden dev build: the store's
loadingfor link clicks, history, 204s, downloads (link and address bar), SPA, hash and iframe polls; reduced motion switched mid-load; no animations left after a load ends or a tab closes mid-load.pnpm lint,pnpm tc,check:code-quality:changed,check:react-doctor:changed,check:zustand-selector-fanout,oxfmt --check, type-aware oxlint, browser-pane and tab-bar unit tests (1765), and the reload-feedback e2e spec all pass on a branch rebased onto main.Mobile: paired an Android emulator to a hidden desktop dev build and drove loads from the desktop (video above), on both the over-the-air page and a native render. Mobile lint, typecheck, test-typecheck ratchet and browser tests pass. No new mobile unit test: the line's only behaviour is its animation, which the mocked test renderer can't show. Five pane tests stub the new component, because their
react-nativemocks have noAnimated.Desktop tested on macOS. The desktop change is renderer-only and uses no platform-specific APIs. iOS not run; the mobile component uses only cross-platform
Animated.AI Disclosure
Review
Codex review, 3 rounds. Round 1 found two things: the doc preview's reload button would have shown an X that reloads, and reduced motion didn't apply mid-load. Both fixed. Rounds 2 and 3 were clean. A fourth round after adding mobile found two real issues in the mobile line, both fixed: a late native
stopAnimationanswer could fade out a newer load, and a reduced-motion change mid-load was ignored. It also asked for headless-server loading, which is the separate follow-up above. A separate design review measured that a long-running animation keeps ticking the main thread in hidden or closed tabs, so the line now holds with a finished animation and is cancelled on stop and unmount.Agent skill upstream boundary
Notes
Animatedwith the native driver on iOS and Android, and JS-driven on the web page (only while loading).loadingrule (Orca-started loads only).loadingboolean to the runtime and mobile. It just stops turning on for iframe loads, so old and new versions work together. No IPC, wire or saved-state change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)