feat: add selectable status indicator to toolbar v2#848
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-7
branch
from
67f3bd1 to
bedf313
Compare
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-8
branch
from
6f798fa to
5885839
Compare
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-7
branch
from
bedf313 to
07cd540
Compare
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-8
branch
from
5885839 to
005eede
Compare
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-7
branch
from
07cd540 to
e2af205
Compare
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-8
branch
from
005eede to
6875fe2
Compare
thomaswhyyou
force-pushed
the
thomas-kno-11142-sdk-guide-toolbar-v2-poc-8
branch
from
7467313 to
5cd2644
Compare
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| result.metadata = { guideGroup: defaultGroup }; | ||
| result.metadata = { guideGroup: defaultGroup, filters, ...metadata }; |
There was a problem hiding this comment.
I knew this metadata field was going to come in handy at some point..! :)
| ); | ||
|
|
||
| const selectedGuide = this.selectGuide(state, filters, opts); | ||
| // 1. First, call selectGuide() using the same filters to ensure we have a |
There was a problem hiding this comment.
No functional changes here, except for adding code paths to record select query results with additional metadata.
By "recording", we accumulate all select query results inside this.stage data. As a reminder, this "group stage" state data is something we manage outside of the tanstack store, which powers guides the priority resolution mechanism in the guide client each time a new page renders).
We are recording select query results w/ metadata, in order for the toolbar to use it as a basis to infer which guides could or could not render. It's a proxy to guessing if/what guide components are present on the current page.
By default we only record query results while in debug mode.
| return undefined; | ||
| } | ||
|
|
||
| const result = select(state, filters); |
There was a problem hiding this comment.
No changes to the core logic here, but it's been moved down because we moved up the part where we open the group stage (i.e. this.openGroupStage()). Previously we could delay opening the group stage until later, but in order record all the select queries being issued in a given page, we need the group stage to be open.
| // Must come AFTER we ensure a group stage exists above, so we can record | ||
| // select queries. By default, we only record the result while in debugging. | ||
| const { recordSelectQuery = !!state.debug?.debugging } = opts; | ||
| const metadata: SelectQueryMetadata = { | ||
| limit: "one", | ||
| opts: { ...opts, recordSelectQuery }, | ||
| }; | ||
| const result = select(state, filters, metadata); | ||
| this.maybeRecordSelectResult(result); |
There was a problem hiding this comment.
This is the new bit, to record query results.
|
|
||
| // Check if inside the throttle window (i.e. throttled) and if so stop and | ||
| // return undefined unless explicitly given the option to include throttled. | ||
| const throttled = !opts.includeThrottled && checkStateIfThrottled(state); |
There was a problem hiding this comment.
Moved the early return to later inside the switch statement below. Previously we didn't need to resolve the group stage while in the throttle window. But now we want the toolbar to still know which guide is being resolved to display but being throttled. Should functionally be the same.
| StoreState, | ||
| } from "./types"; | ||
|
|
||
| // Extends the map class to allow having metadata on it, which is used to record |
There was a problem hiding this comment.
Moved to types.ts to make the imports a bit more streamlined.
…tAllByTypeReturnStatus logic
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| const byKey = <T extends { key: string }>(items: T[]) => { | ||
| return items.reduce((acc, item) => ({ ...acc, [item.key]: item }), {}); | ||
| }; |
There was a problem hiding this comment.
Duplicated byKey utility already exists in helpers
Low Severity
The byKey helper duplicates the identical function already exported from packages/client/src/clients/guide/helpers.ts. It isn't currently re-exported from the package index, but adding it to the index.ts exports (alongside the newly added checkStateIfThrottled) would avoid the duplication.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #848 +/- ##
==========================================
+ Coverage 67.41% 67.90% +0.49%
==========================================
Files 204 204
Lines 8561 8767 +206
Branches 1118 1183 +65
==========================================
+ Hits 5771 5953 +182
- Misses 2766 2790 +24
Partials 24 24
|
2 participants
Description
Another follow up to #846 and #847, this PR adds a "selectable" status to guide's annotation, and surfaced in
GuideRow. This should be the final status indicator, which combined with other statuses should allow the toolbar to answer the question "why am i seeing or not seeing guide X"?The "selectable" status is a bit more involved than other statuses because: