-
Notifications
You must be signed in to change notification settings - Fork 212
feat: Add UI for Start with a Query Flow CLOUDP-311785 #6881
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
base: main
Are you sure you want to change the base?
Conversation
packages/compass-indexes/src/components/create-index-form/query-flow-section.tsx
Outdated
Show resolved
Hide resolved
<div> | ||
<CodemirrorInlineEditor | ||
data-testid="query-flow-section-json-editor" | ||
language="json" |
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.
We want the language here to be language="javascript-expression"
so that it accepts shell syntax like
{
_id: ObjectId("66db48fd73f134799d529645")
}
That's the default language of this property of the component so a change we can do is remove this line:
language="json" |
We should also remove the initialJSONFoldAll
prop then too and update the data-testid
to not mention json.
</Body> | ||
<div className={inputQueryContainerStyles}> | ||
<div> | ||
<CodemirrorInlineEditor |
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.
Do we want to have field autocompletion here? Here's an example of where we do that elsewhere:
completer={completer} |
The
createQueryAutocompleter
function is the one that's useful there.
completer={createQueryAutocompleter({ | ||
fields: schemaFields, | ||
serverVersion, | ||
})} |
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.
Can we move the createQueryAutocompleter
into a useMemo
? That way we don't re-create the auto completer on each render:
const completer = useMemo(() => createQueryAutocompleter({
fields: schemaFields,
serverVersion,
}), [schemaFields, serverVersion]);
}} | ||
className={suggestedIndexButtonStyles} | ||
> | ||
Show me suggested index |
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.
Maybe Show suggested index
instead?
Show me suggested index | |
Show suggested index |
</Body> | ||
<div className={inputQueryContainerStyles}> | ||
<div> | ||
<CodemirrorInlineEditor |
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.
We should use the CodemirrorMultilineEditor
instead of the Inline Editor here. The inline editor is designed for places like the query bar where the query is typically in one line.
marginBottom: spacing[600], | ||
border: `1px solid ${palette.gray.base}`, | ||
borderRadius: spacing[300], | ||
padding: spacing[600], |
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.
Having the padding here makes it so that the editor can't be focused when the user clicks on the margin. We should have it so that the padding here is actually in the editor.
It's a bit hacky, but we can do:
const editorStyles = css({
'& .cm-content': {
padding: spacing[600]
}
});
and pass the editorStyles
as the className
to the CodemirrorMultilineEditor
.
Input Query | ||
</Body> | ||
<div className={inputQueryContainerStyles}> | ||
<div> |
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.
We should show a focus ring when the user is in the editor. It's mostly for accessibility, showing that the user is focused on that element.
We can do that by passing the focus ring classname:
const focusRingProps = useFocusRing({
outer: true,
focusWithin: true,
hover: true,
});
{...}
<div className={focusRingProps.className}>
We do something similar in the option-editor:
const focusRingProps = useFocusRing({ |
Description
Added ui without functionality for the query flow

active state

Checklist
Motivation and Context
Open Questions
Dependents
Types of changes