-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add selectable status indicator to toolbar v2 #848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
697b2bc
369a479
945fcb7
b8b62a4
7a81c43
fafa212
92a4ba3
ba59e93
a52b6f7
3a819a5
5bb486b
9e426c6
6dd3dd4
91621da
4f36746
616b795
6426195
d5f2c43
f57dd1c
d8ddfe2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,6 @@ import Knock from "../../knock"; | |
|
|
||
| import { | ||
| DEFAULT_GROUP_KEY, | ||
| SelectionResult, | ||
| byKey, | ||
| checkStateIfThrottled, | ||
| findDefaultGroup, | ||
|
|
@@ -46,6 +45,8 @@ import { | |
| SelectFilterParams, | ||
| SelectGuideOpts, | ||
| SelectGuidesOpts, | ||
| SelectQueryLimit, | ||
| SelectionResult, | ||
| StepMessageState, | ||
| StoreState, | ||
| TargetParams, | ||
|
|
@@ -150,7 +151,16 @@ const safeJsonParseDebugParams = (value: string): DebugState => { | |
| } | ||
| }; | ||
|
|
||
| const select = (state: StoreState, filters: SelectFilterParams = {}) => { | ||
| type SelectQueryMetadata = { | ||
| limit: SelectQueryLimit; | ||
| opts: SelectGuideOpts; | ||
| }; | ||
|
|
||
| const select = ( | ||
| state: StoreState, | ||
| filters: SelectFilterParams, | ||
| metadata: SelectQueryMetadata, | ||
| ) => { | ||
| // A map of selected guides as values, with its order index as keys. | ||
| const result = new SelectionResult(); | ||
|
|
||
|
|
@@ -175,7 +185,8 @@ const select = (state: StoreState, filters: SelectFilterParams = {}) => { | |
| result.set(index, guide); | ||
| } | ||
|
|
||
| result.metadata = { guideGroup: defaultGroup }; | ||
| result.metadata = { guideGroup: defaultGroup, filters, ...metadata }; | ||
|
|
||
| return result; | ||
| }; | ||
|
|
||
|
|
@@ -617,14 +628,35 @@ export class KnockGuideClient { | |
| `[Guide] .selectGuides (filters: ${formatFilters(filters)}; state: ${formatState(state)})`, | ||
| ); | ||
|
|
||
| const selectedGuide = this.selectGuide(state, filters, opts); | ||
| if (!selectedGuide) { | ||
| // 1. First, call selectGuide() using the same filters to ensure we have a | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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. |
||
| // group stage open and respect throttling. This isn't the real query, but | ||
| // rather it's a shortcut ahead of handling the actual query result below. | ||
| const selectedGuide = this.selectGuide(state, filters, { | ||
| ...opts, | ||
| // Don't record this result, not the actual query result we need. | ||
| recordSelectQuery: false, | ||
| }); | ||
|
|
||
| // 2. Now make the actual select query with the provided filters and opts, | ||
| // and record the result (as needed). By default, we only record the result | ||
| // while in debugging. | ||
| const { recordSelectQuery = !!state.debug?.debugging } = opts; | ||
| const metadata: SelectQueryMetadata = { | ||
| limit: "all", | ||
| opts: { ...opts, recordSelectQuery }, | ||
| }; | ||
| const result = select(state, filters, metadata); | ||
| this.maybeRecordSelectResult(result); | ||
|
|
||
| // 3. Stop if there is not at least one guide to return. | ||
| if (!selectedGuide && !opts.includeThrottled) { | ||
| return []; | ||
| } | ||
|
|
||
| // There should be at least one guide to return here now. | ||
| const guides = [...select(state, filters).values()]; | ||
| const guides = [...result.values()]; | ||
|
|
||
| // 4. If throttled, filter out any throttled guides. | ||
| if (!opts.includeThrottled && checkStateIfThrottled(state)) { | ||
| const unthrottledGuides = guides.filter( | ||
| (g) => g.bypass_global_group_limit, | ||
|
|
@@ -657,32 +689,6 @@ export class KnockGuideClient { | |
| return undefined; | ||
| } | ||
|
|
||
| const result = select(state, filters); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| if (result.size === 0) { | ||
| this.knock.log("[Guide] Selection found zero result"); | ||
| return undefined; | ||
| } | ||
|
|
||
| const [index, guide] = [...result][0]!; | ||
| this.knock.log( | ||
| `[Guide] Selection found: \`${guide.key}\` (total: ${result.size})`, | ||
| ); | ||
|
|
||
| // If a guide ignores the group limit, then return immediately to render | ||
| // always. | ||
| if (guide.bypass_global_group_limit) { | ||
| this.knock.log(`[Guide] Returning the unthrottled guide: ${guide.key}`); | ||
| return guide; | ||
| } | ||
|
|
||
| // Check if inside the throttle window (i.e. throttled) and if so stop and | ||
| // return undefined unless explicitly given the option to include throttled. | ||
| if (!opts.includeThrottled && checkStateIfThrottled(state)) { | ||
| this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`); | ||
| return undefined; | ||
| } | ||
|
|
||
| // Starting here to the end of this method represents the core logic of how | ||
| // "group stage" works. It provides a mechanism for 1) figuring out which | ||
| // guide components are about to render on a page, 2) determining which | ||
|
|
@@ -716,6 +722,35 @@ export class KnockGuideClient { | |
| this.stage = this.openGroupStage(); // Assign here to make tsc happy | ||
| } | ||
|
|
||
| // 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); | ||
|
Comment on lines
+725
to
+733
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the new bit, to record query results. |
||
|
|
||
| if (result.size === 0) { | ||
| this.knock.log("[Guide] Selection found zero result"); | ||
| return undefined; | ||
| } | ||
|
|
||
| const [index, guide] = [...result][0]!; | ||
| this.knock.log( | ||
| `[Guide] Selection found: \`${guide.key}\` (total: ${result.size})`, | ||
| ); | ||
|
|
||
| // If a guide ignores the group limit, then return immediately to render | ||
| // always. | ||
| if (guide.bypass_global_group_limit) { | ||
| this.knock.log(`[Guide] Returning the unthrottled guide: ${guide.key}`); | ||
| return guide; | ||
| } | ||
|
|
||
| const throttled = !opts.includeThrottled && checkStateIfThrottled(state); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| switch (this.stage.status) { | ||
| case "open": { | ||
| this.knock.log(`[Guide] Adding to the group stage: ${guide.key}`); | ||
|
|
@@ -725,8 +760,16 @@ export class KnockGuideClient { | |
|
|
||
| case "patch": { | ||
| this.knock.log(`[Guide] Patching the group stage: ${guide.key}`); | ||
| // Refresh the ordered queue in the group stage while continuing to | ||
| // render the currently resolved guide while in patch window, so that | ||
| // we can re-resolve when the group stage closes. | ||
| this.stage.ordered[index] = guide.key; | ||
|
|
||
| if (throttled) { | ||
| this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`); | ||
| return undefined; | ||
| } | ||
|
|
||
| const ret = this.stage.resolved === guide.key ? guide : undefined; | ||
| this.knock.log( | ||
| `[Guide] Returning \`${ret?.key}\` (stage: ${formatGroupStage(this.stage)})`, | ||
|
|
@@ -735,6 +778,11 @@ export class KnockGuideClient { | |
| } | ||
|
|
||
| case "closed": { | ||
| if (throttled) { | ||
| this.knock.log(`[Guide] Throttling the selected guide: ${guide.key}`); | ||
| return undefined; | ||
| } | ||
|
|
||
| const ret = this.stage.resolved === guide.key ? guide : undefined; | ||
| this.knock.log( | ||
| `[Guide] Returning \`${ret?.key}\` (stage: ${formatGroupStage(this.stage)})`, | ||
|
|
@@ -744,6 +792,42 @@ export class KnockGuideClient { | |
| } | ||
| } | ||
|
|
||
| // Record select query results by accumulating them by 1) key or type first, | ||
| // and then 2) "one" or "all". | ||
| private maybeRecordSelectResult(result: SelectionResult) { | ||
| if (!result.metadata) return; | ||
|
|
||
| const { opts, filters, limit } = result.metadata; | ||
| if (!opts.recordSelectQuery) return; | ||
| if (!filters.key && !filters.type) return; | ||
| if (!this.stage || this.stage.status === "closed") return; | ||
|
|
||
| // Deep merge to accumulate the results. | ||
| const queriedByKey = this.stage.results.key || {}; | ||
| if (filters.key) { | ||
| queriedByKey[filters.key] = { | ||
| ...(queriedByKey[filters.key] || {}), | ||
| ...{ [limit]: result }, | ||
| }; | ||
| } | ||
| const queriedByType = this.stage.results.type || {}; | ||
| if (filters.type) { | ||
| queriedByType[filters.type] = { | ||
| ...(queriedByType[filters.type] || {}), | ||
| ...{ [limit]: result }, | ||
| }; | ||
| } | ||
|
|
||
| this.stage = { | ||
| ...this.stage, | ||
| results: { key: queriedByKey, type: queriedByType }, | ||
| }; | ||
| } | ||
|
|
||
| getStage() { | ||
| return this.stage; | ||
| } | ||
|
|
||
| private openGroupStage() { | ||
| this.knock.log("[Guide] Opening a new group stage"); | ||
|
|
||
|
|
@@ -759,6 +843,7 @@ export class KnockGuideClient { | |
| this.stage = { | ||
| status: "open", | ||
| ordered: [], | ||
| results: {}, | ||
| timeoutId, | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,23 +3,11 @@ import { | |
| GuideActivationUrlRuleData, | ||
| GuideData, | ||
| GuideGroupData, | ||
| KnockGuide, | ||
| KnockGuideActivationUrlPattern, | ||
| SelectFilterParams, | ||
| StoreState, | ||
| } from "./types"; | ||
|
|
||
| // Extends the map class to allow having metadata on it, which is used to record | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to types.ts to make the imports a bit more streamlined. |
||
| // the guide group context for the selection result (though currently only a | ||
| // default global group is supported). | ||
| export class SelectionResult<K = number, V = KnockGuide> extends Map<K, V> { | ||
| metadata: { guideGroup: GuideGroupData } | undefined; | ||
|
|
||
| constructor() { | ||
| super(); | ||
| } | ||
| } | ||
|
|
||
| export const formatGroupStage = (stage: GroupStage) => { | ||
| return `status=${stage.status}, resolved=${stage.resolved}`; | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I knew this metadata field was going to come in handy at some point..! :)