Add loading count source for http requests#59245
Conversation
|
Pinging @elastic/kibana-platform (Team:Platform) |
| const fetchService = new Fetch({ basePath, kibanaVersion }); | ||
| const loadingCount = this.loadingCount.setup({ fatalErrors }); | ||
| loadingCount.addLoadingCountSource(fetchService.getRequestCount$()); |
There was a problem hiding this comment.
NIT: could add a test to ensure this is properly called.
There was a problem hiding this comment.
I added a test, but to avoid some really complex mocking I just asserted that addLoadingCountSource is called with an Observable instance, not specifically the one from fetchService.getRequestCount$. WDYT?
There was a problem hiding this comment.
Sorry, was under water last week on the SO management, did not see the notif. SGTM.
4d1709d to
df7f91e
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
|
To ease the platform migration I think it's useful that we match the legacy behaviour. But I think we should add the ability to opt-out of increasing the loading count. Users don't need to be made aware of all in-progress http requests. Showing a loading bar when we're sending usage metrics to the server or polling for updates from the pulse newsfeed would probably be distracting or confusing to users. |
…re/files-and-filetree * 'master' of github.com:elastic/kibana: (254 commits) Convert discover_page to ts, remove redundunt methods (elastic#59312) [Fix for Vis Editor] Revert setting time field to empty string when it's undefined (elastic#58873) Delete legacy search endpoint (elastic#59341) [Uptime] Improve duration chart (elastic#58404) [Snapshot & Restore] NP migration (elastic#59109) [ML] Add support for date_nanos time field in anomaly job wizard (elastic#59017) Revert "Makes alerting and actions optional properties for interface RequestH… (elastic#59264)" Change remote_clusters ID to remoteClusters (elastic#59246) Makes alerting and actions optional properties for interface RequestH… (elastic#59264) Clean up date histogram agg type. (elastic#58805) [ML] Management: fix license unsubscribe (elastic#59365) Remove documentation for server.cors settings (elastic#59096) Edit alert flyout (elastic#58964) [SIEM] Fix rule delete/duplicate actions (elastic#59306) move mouse to close obstructing tooltip (elastic#59214) Reset page after deleting (elastic#59310) Make sure phrases input filter triggers autosuggestons (elastic#59299) Add loading count source for http requests (elastic#59245) Revert "[ML] Transforms: Deprecate custom KibanaContext. (elastic#59133)" Expose metrics service to public API (elastic#59294) ... # Conflicts: # src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
@rudolf That's a good point. We should probably opt out any requests that are using the |
Summary
Fixes #59222
Adds an observable to the internal Fetch class that gets registered with the
core.http.addLoadingCount$API so that any pending HTTP requests make the global loading bar appear.Checklist
Delete any items that are not applicable to this PR.
For maintainers