diff --git a/.changeset/tidy-toys-lie.md b/.changeset/tidy-toys-lie.md new file mode 100644 index 00000000..e5328eca --- /dev/null +++ b/.changeset/tidy-toys-lie.md @@ -0,0 +1,5 @@ +--- +"@guardian/cql": minor +--- + +Make suggestions position-dependent (rather than surfacing all possible positions for a query), and add `showTypeaheadForQueryStr`, enabling typeahead suggestions in queryStr positions diff --git a/README.md b/README.md index 396917eb..6a02251a 100644 --- a/README.md +++ b/README.md @@ -88,4 +88,4 @@ This repository uses [changesets](https://github.com/changesets/changesets) for To release a new version with your changes, run `bun changeset add` and follow the prompts. This will create a new changeset file in the .changeset directory. Commit this file with your PR. -When your PR is merged, Changesets will create a PR to release the new version. +When your PR is merged, Changesets will create a PR to release the new version. Please feel free to merge a PR that has been generated by a PR you have merged. diff --git a/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts b/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts index 4162c965..e3d7bbb7 100644 --- a/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts +++ b/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts @@ -35,20 +35,24 @@ import { TestTypeaheadHelpers } from "../../../lang/fixtures/TestTypeaheadHelper import { isVisibleDataAttr } from "../../popover/Popover"; import { docToCqlStrWithSelection, tick } from "../../../utils/test"; import { createParser } from "../../../lang/Cql"; -import { Typeahead } from "../../../lang/typeahead"; +import { Typeahead, TypeaheadConfig } from "../../../lang/typeahead"; import { chip, chipValue, IS_SELECTED } from "../schema"; import { Node, NodeType } from "prosemirror-model"; import { cqlQueryStrFromQueryAst } from "../../../lang/interpreter"; import { EditorView } from "prosemirror-view"; import { TokenType } from "../../../lang/token"; -const typeheadHelpers = new TestTypeaheadHelpers(); -const testCqlService = new Typeahead(typeheadHelpers.typeaheadFields); - const createCqlEditor = ( initialQuery: string = "", config: CqlConfig = { syntaxHighlighting: true }, + typeaheadConfig: Partial = {}, ) => { + const typeheadHelpers = new TestTypeaheadHelpers(); + const testCqlService = new Typeahead( + typeheadHelpers.typeaheadFields, + typeaheadConfig, + ); + document.body.innerHTML = ""; const container = document.body; const typeaheadEl = document.createElement("div"); @@ -221,6 +225,39 @@ describe("cql plugin", () => { }); describe("typeahead", () => { + describe("queryStr", () => { + const createCqlEditorWithQueryStrTypeahead = ( + initialQuery: string = "", + ) => + createCqlEditor( + initialQuery, + {}, + { showTypeaheadForQueryStr: true, minCharsForQueryStrTypeahead: 2 }, + ); + it("does not display a popover at the start of the query when fewer than the minimum chars necessary for a queryStr suggestion are added", async () => { + const { container } = createCqlEditorWithQueryStrTypeahead("t"); + + await assertPopoverVisibility(container, false); + }); + + it("does display a popover at the start of the query when the minimum chars necessary for a queryStr suggestion are added", async () => { + const { container } = createCqlEditorWithQueryStrTypeahead("ta"); + + await assertPopoverVisibility(container, true); + }); + + it("accepts a suggestion from a popover, and applies a chipKey", async () => { + const { editor, container, waitFor } = + createCqlEditorWithQueryStrTypeahead("ta"); + + await selectPopoverOptionWithEnter(editor, container, "Tag"); + const nodeAtCaret = getNodeTypeAtSelection(editor.view); + expect(nodeAtCaret.name).toBe("chipValue"); + + await waitFor("tag:"); + }); + }); + describe("chip keys", () => { it("displays a colon between chip keys and values on first render", async () => { const queryStr = "+x:y"; @@ -443,10 +480,10 @@ describe("cql plugin", () => { await findByText(popoverContainer, "Tags are magic"); }); - it("applies the given key when a popover option is selected", async () => { + it("applies the given value when a popover option is selected", async () => { const queryStr = "example +tag:"; const { editor, container, waitFor, moveCaretToQueryPos } = - createCqlEditor("example +tag:"); + createCqlEditor(queryStr); await moveCaretToQueryPos(queryStr.length); await editor.insertText("t"); @@ -459,6 +496,23 @@ describe("cql plugin", () => { await waitFor("example tag:tags-are-magic"); }); + it("respects the whitespace after the value after applying", async () => { + const queryStr = "+tag: example"; + const { editor, container, waitFor, moveCaretToQueryPos } = + createCqlEditor(queryStr); + + await moveCaretToQueryPos(queryStr.indexOf(" ")); + await editor.insertText("t"); + + await selectPopoverOptionWithEnter( + editor, + container, + "Tags are magic", + ); + + await waitFor("tag:tags-are-magic example"); + }); + it("applies the given quoted suggestion in value position when it contains whitespace", async () => { const queryStr = "example +tag:"; const { editor, container, waitFor, moveCaretToQueryPos } = diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index 72c3f6f8..18dc359d 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -100,7 +100,6 @@ const getFieldValueRanges = ( literalOffsetStart: number, literalOffsetEnd: number, ): [number, number, number][] => { - return [ [from, 0, 1 /* end / start */], [from, literalOffsetStart, 0], @@ -589,7 +588,7 @@ export const toMappedSuggestions = ( } const from = mapping.map(typeaheadSuggestion.from); const to = mapping.map( - typeaheadSuggestion.to + 1, + typeaheadSuggestion.to, typeaheadSuggestion.position === "chipKey" ? -1 : 0, ); @@ -630,18 +629,12 @@ export const getNextPositionAfterTypeaheadSelection = ( ); if (nodeTypeAfterIndex === -1) { - console.warn( - `Attempted to find a selection, but the position ${currentPos} w/in node ${suggestionNode.type.name} is not one of ${typeaheadSelectionSequence.map((_) => _.name).join(",")}`, - ); return; } const nodeTypeToSelect = typeaheadSelectionSequence[nodeTypeAfterIndex + 1]; if (!nodeTypeToSelect) { - console.warn( - `Attempted to find a selection, but the position ${currentPos} w/in node ${suggestionNode.type.name} does not have anything to follow a node of type ${nodeTypeAfterIndex}`, - ); return; } @@ -744,9 +737,17 @@ export const applyChipLifecycleRules = (tr: Transaction): void => { }; export const applySuggestion = - (view: EditorView) => (from: number, to: number, value: string) => { + (view: EditorView) => + ( + from: number, + to: number, + position: TypeaheadSuggestion["position"], + _value: string, + ) => { const tr = view.state.tr; + const value = position === "queryStr" ? `${_value}:` : _value; + tr.replaceRangeWith(from, to, schema.text(value)).setMeta( TRANSACTION_APPLY_SUGGESTION, true, @@ -785,7 +786,7 @@ export const handleEnter = (view: EditorView) => { const keyText = `${maybePrecedingWhitespace}${maybePolarity}${fromNode.textContent}`; const chipStart = $from.before() - 1; - const chipEnd = endOfKey + 4; // +1 to move to start of value, +1 to move to end of value, +1 to + const chipEnd = endOfKey + 4; // +1 to move to start of value, +1 to move to end of value const endOfPrecedingQueryStr = chipStart - 1; const tr = state.tr; diff --git a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts index 2ad694dc..01733560 100644 --- a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts +++ b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts @@ -48,7 +48,12 @@ export class TypeaheadPopover extends Popover { private updateRendererState: (state: PopoverRendererState) => void = noopUpdateRendererState; - private _applySuggestion: (from: number, to: number, value: string) => void; + private _applySuggestion: ( + from: number, + to: number, + position: TypeaheadSuggestion["position"], + value: string, + ) => void; private _skipSuggestion: () => void; private currentSuggestion: TypeaheadSuggestion | undefined; private currentOptionIndex = 0; @@ -58,7 +63,12 @@ export class TypeaheadPopover extends Popover { private view: EditorView, protected popoverEl: HTMLElement, // Apply a suggestion to the input, replacing the given range - applySuggestion: (from: number, to: number, value: string) => void, + applySuggestion: ( + from: number, + to: number, + position: TypeaheadSuggestion["position"], + value: string, + ) => void, // Skip a suggestion, and move on to the next valid field skipSuggestion: () => void, // A callback that receives everything necessary to render popover content @@ -201,7 +211,7 @@ export class TypeaheadPopover extends Popover { this.hide(); } - this._applySuggestion(from, to, value); + this._applySuggestion(from, to, position, value); }; private skipSuggestion = () => { diff --git a/lib/cql/src/lang/typeahead.ts b/lib/cql/src/lang/typeahead.ts index 99d4270c..47ece868 100644 --- a/lib/cql/src/lang/typeahead.ts +++ b/lib/cql/src/lang/typeahead.ts @@ -1,4 +1,5 @@ import { ProseMirrorToken } from "../cqlInput/editor/utils"; +import { mergeDeep } from "../utils/merge"; import { CqlQuery } from "./ast"; import { DateSuggestionOption, @@ -21,12 +22,13 @@ const compareValueAndLabel = const filterAndSortTextSuggestionOption = ( suggestions: TextSuggestionOption[], str: string, + onlyIncludeStartsWith: boolean = false ) => { const lowerCaseStr = str.toLowerCase(); return suggestions .filter( compareValueAndLabel(lowerCaseStr, (str, compare) => - str.includes(compare), + onlyIncludeStartsWith ? str.startsWith(compare) : str.includes(compare), ), ) .sort((a, b) => { @@ -71,18 +73,49 @@ export class TypeaheadField { } } +export type TypeaheadConfig = { + showTypeaheadForQueryStr: boolean; + minCharsForQueryStrTypeahead: number; +}; + +const defaultTypeaheadConfig: TypeaheadConfig = { + showTypeaheadForQueryStr: false, + minCharsForQueryStrTypeahead: 2, +}; + export class Typeahead { private typeaheadFieldEntries: TextSuggestionOption[]; private abortController: AbortController | undefined; + private config: TypeaheadConfig; - constructor(private typeaheadFields: TypeaheadField[]) { + constructor( + private typeaheadFields: TypeaheadField[], + config: Partial = {}, + ) { + this.config = mergeDeep(defaultTypeaheadConfig, config); this.typeaheadFieldEntries = this.typeaheadFields.map((field) => field.toSuggestionOption(), ); } + /** + * Get suggestions for the given query and position. + * + * `position` is a caret position, 0-indexed from the start of the string, + * where every character has a before and after. For example, the query + * + * `s t r k : v` + * | | | | | | | | + * 0 1 2 3 4 5 6 7 + * + * would give suggestions for: + * - keys containing `str` for positions 0-3, if `showTypeaheadForQueryStr` + * was `true` + * - keys containing `k` for positions 4-5 + * - keys containing `v` for positions 6-7 + */ public async getSuggestions( - program: CqlQuery, + query: CqlQuery, position: number, signal?: AbortSignal, ): Promise { @@ -90,7 +123,7 @@ export class Typeahead { // Abort existing fetch, if it exists this.abortController?.abort(); - if (!program.content) { + if (!query.content) { return resolve(undefined); } @@ -100,9 +133,15 @@ export class Typeahead { reject(new DOMException("Aborted", "AbortError")); }); - const maybeSuggestionAtPos = getAstNodeAtPos(program.content, position); + const maybeSuggestionAtPos = getAstNodeAtPos(query.content, position); + + const isValidSuggestionNode = + maybeSuggestionAtPos?.key.tokenType !== "STRING" || + (this.config.showTypeaheadForQueryStr && + (maybeSuggestionAtPos?.key.literal?.length ?? 0) >= + this.config.minCharsForQueryStrTypeahead); - if (!maybeSuggestionAtPos) { + if (!maybeSuggestionAtPos || !isValidSuggestionNode) { return resolve(undefined); } @@ -124,7 +163,7 @@ export class Typeahead { return { from: keyToken.from, to: Math.max(keyToken.to, keyToken.to - 1), // Do not include ':' - position: "chipKey", + position: keyToken.tokenType === "STRING" ? "queryStr" : "chipKey", suggestions, type: "TEXT", suffix: ":", @@ -153,8 +192,8 @@ export class Typeahead { const suggestions = await maybeValueSuggestions.suggestions; return { - from: value ? value.from : key.from, - to: value ? value.to : key.to, + from: value.from, + to: value.to, position: "chipValue", suggestions, type: maybeValueSuggestions.type, @@ -170,6 +209,7 @@ export class Typeahead { const suggestions = filterAndSortTextSuggestionOption( this.typeaheadFieldEntries, str, + this.config.showTypeaheadForQueryStr ); if (suggestions.length) { diff --git a/lib/cql/src/lang/types.ts b/lib/cql/src/lang/types.ts index 4ce94abf..3058d4da 100644 --- a/lib/cql/src/lang/types.ts +++ b/lib/cql/src/lang/types.ts @@ -1,4 +1,5 @@ type BaseSuggestion = { + // These are ProseMirror positions readonly from: number; readonly to: number; readonly position: "queryStr" | "chipKey" | "chipValue"; diff --git a/lib/cql/src/lang/utils.ts b/lib/cql/src/lang/utils.ts index 798b48ef..a5153b4e 100644 --- a/lib/cql/src/lang/utils.ts +++ b/lib/cql/src/lang/utils.ts @@ -30,7 +30,7 @@ export function getNPermutations(arr: T[], n: number): T[][] { permutations.push(next.value); } } - return permutations + return permutations; } export function* getPermutations( @@ -111,15 +111,17 @@ export const getAstNodeAtPosExpr = ( ? { key, // Add an empty value here to signal that we are in value position - value: toProseMirrorToken( - new Token( + value: { + ...new Token( TokenType.CHIP_VALUE, "", undefined, position, position, ), - ), + from: position, + to: position, + }, } : { key, value: undefined }; } diff --git a/lib/cql/src/page.ts b/lib/cql/src/page.ts index 668b7d41..8bc70ca2 100644 --- a/lib/cql/src/page.ts +++ b/lib/cql/src/page.ts @@ -8,9 +8,9 @@ import { } from "./cqlInput/editor/debug.ts"; import { createParser } from "./lang/Cql.ts"; import { Typeahead, TypeaheadField } from "./lang/typeahead.ts"; -import { CapiTypeaheadProvider } from "./typeahead/CapiTypeaheadHelpers.ts"; import { toolsSuggestionOptionResolvers } from "./typeahead/tools-index/config"; import { DebugChangeEventDetail, QueryChangeEventDetail } from "./types/dom"; +import { CapiTypeaheadProvider } from "./lib.ts"; const setUrlParam = (key: string, value: string) => { const urlParams = new URLSearchParams(window.location.search); @@ -135,7 +135,10 @@ const typeaheadHelpersCapi = new CapiTypeaheadProvider( initialEndpointCapi, "test", ); -const capiTypeahead = new Typeahead(typeaheadHelpersCapi.typeaheadFields); +const capiTypeahead = new Typeahead(typeaheadHelpersCapi.typeaheadFields, { + showTypeaheadForQueryStr: true, + minCharsForQueryStrTypeahead: 2, +}); const CqlInputCapi = createCqlInput(capiTypeahead, { syntaxHighlighting: true,