Skip to content
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

Text Command Popover Fixes/UI Improvements #7728

Merged
merged 15 commits into from
Feb 26, 2024
Merged
44 changes: 11 additions & 33 deletions src/bricks/effects/InsertAtCursorEffect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ import {
} from "@/types/runtimeTypes";
import { type Schema } from "@/types/schemaTypes";
import { propertiesToSchema } from "@/validators/generic";
import textFieldEdit from "text-field-edit";
import { BusinessError } from "@/errors/businessErrors";
import { isEmpty } from "lodash";
import focus from "@/utils/focusController";

import { isNativeField } from "@/types/inputTypes";
import type { PlatformCapability } from "@/platform/capabilities";
import { insertAtCursorWithCustomEditorSupport } from "@/contentScript/textEditorDom";

/**
* Insert text at the cursor position. For use with text snippets, etc.
Expand All @@ -52,6 +51,11 @@ class InsertAtCursorEffect extends EffectABC {
["text"],
);

override async getRequiredCapabilities(): Promise<PlatformCapability[]> {
// Requires pageScript to support editors like CKEditor where we need to use their editor API
return ["dom", "contentScript", "pageScript"];
}

override async isRootAware(): Promise<boolean> {
return true;
}
Expand All @@ -70,36 +74,10 @@ class InsertAtCursorEffect extends EffectABC {
throw new BusinessError("No active element");
}

// Demo page: https://pbx.vercel.app/bootstrap-5/
if (isNativeField(element)) {
textFieldEdit.insert(element, text);
return;
}

// Reference editors to check:
// - ✅ Vanilla content editable: https://pbx.vercel.app/react-admin/#/products/1/description
// - ⚠️ DraftJS: https://draftjs.org/ - doesn't advance the cursor
// - ⚠️ TinyMCE: https://www.tiny.cloud/docs/demo/basic-example/ - when using Run All Frames, doesn't advance cursor
// - ❌ CKEditor: https://ckeditor.com/ckeditor-5/demo/feature-rich/
if (element.contentEditable) {
// Ensure window is focused so, so that when calling from the sidebar, the browser will show the cursor and
// the user can keep typing
window.focus();

// Ensure the element has focus, so that text is inserted at the cursor position
if (document.activeElement !== element) {
element.focus();
}

// Using document.execCommand seems to be more reliable than range.insertNode(document.createTextNode(text));
document.execCommand("insertText", false, text);

return;
}

throw new BusinessError(
"Target element is not an input, textarea, or contenteditable element.",
);
await insertAtCursorWithCustomEditorSupport({
element,
text,
});
}
}

Expand Down
38 changes: 31 additions & 7 deletions src/contentScript/commandPopover/CommandPopover.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,29 @@

@import "@/themes/colors.scss";

.root {
height: 100px;
width: 200px;
overflow-y: auto;
}

.results {
display: flex;
flex-direction: column;
background-color: $N0;

height: 100px;
width: 200px;

overflow-y: auto;
color: black;
}

.result {
text-align: left;
display: block;
text-align: left;

background-color: $N0;
border: 0;
padding: 0.5em 0.5em;

color: black;
background-color: $N0;

&--selected {
background-color: $P100;
}
Expand All @@ -52,3 +56,23 @@
font-weight: bold;
}
}

.status {
display: block;

padding: 0.5em 0.5em;

font-size: 0.85em;

&--error {
color: #dc3545;
}

&--empty {
color: $N400;
}

&--fetching {
color: $N400;
}
}
7 changes: 6 additions & 1 deletion src/contentScript/commandPopover/CommandPopover.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export default {
component: CommandPopover,
} as ComponentMeta<typeof CommandPopover>;

const Template: ComponentStory<typeof CommandPopover> = ({ registry }) => {
const Template: ComponentStory<typeof CommandPopover> = ({
registry,
commandKey,
}) => {
const elementRef = useRef(null);
const [element, setElement] = useState<Nullishable<HTMLElement>>(null);

Expand All @@ -46,6 +49,7 @@ const Template: ComponentStory<typeof CommandPopover> = ({ registry }) => {
{element && (
<div>
<CommandPopover
commandKey={commandKey}
registry={registry}
element={element}
onHide={action("hide")}
Expand Down Expand Up @@ -104,6 +108,7 @@ commandRegistry.register(slowErrorCommand);
*/
export const Demo = Template.bind({});
Demo.args = {
commandKey: "\\",
registry: commandRegistry,
onHide: action("onHide"),
// XXX: fix Storybook parameter type instead of passing undefined
Expand Down
72 changes: 42 additions & 30 deletions src/contentScript/commandPopover/CommandPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ import {
popoverSlice,
selectSelectedCommand,
} from "@/contentScript/commandPopover/commandPopoverSlice";
import { getElementText, replaceAtCommand } from "@/utils/editorUtils";
import { isEmpty, truncate } from "lodash";
import { getErrorMessage } from "@/errors/errorHelpers";
import { getElementText } from "@/utils/editorUtils";
import { isEmpty } from "lodash";
import reportEvent from "@/telemetry/reportEvent";
import { Events } from "@/telemetry/events";
import EmotionShadowRoot from "react-shadow/emotion";
import { Stylesheets } from "@/components/Stylesheets";
import type { TextCommand } from "@/platform/platformProtocol";
import useIsMounted from "@/hooks/useIsMounted";
import { replaceAtCommand } from "@/contentScript/commandPopover/commandUtils";

// "Every property exists" (via Proxy), TypeScript doesn't offer such type
// Also strictNullChecks config mismatch
Expand Down Expand Up @@ -73,6 +74,7 @@ const ResultItem: React.FunctionComponent<{
}> = ({ isSelected, disabled, command, onClick, query, commandKey }) => {
const elementRef = useRef<HTMLButtonElement>(null);

// Auto-scroll as the user navigates with the arrow keys
useLayoutEffect(() => {
if (isSelected) {
elementRef.current?.scrollIntoViewIfNeeded();
Expand Down Expand Up @@ -101,41 +103,45 @@ const ResultItem: React.FunctionComponent<{

const CommandPopover: React.FunctionComponent<
{
commandKey?: string;
commandKey: string;
registry: CommandRegistry;
element: TextEditorElement;
} & PopoverActionCallbacks
> = ({ commandKey = "/", registry, element, onHide }) => {
> = ({ commandKey, registry, element, onHide }) => {
const isMounted = useIsMounted();
const [state, dispatch] = useReducer(popoverSlice.reducer, initialState);
const selectedCommand = selectSelectedCommand(state);
const selectedCommandRef = useRef(selectedCommand);
const commands = useCommandRegistry(registry);

const fillAtCursor = useCallback(
async (command: TextCommand) => {
async ({ command, query }: { command: TextCommand; query: string }) => {
// Async thunks don't work with React useReducer so write async logic as a hook
// https://github.com/reduxjs/redux-toolkit/issues/754
dispatch(popoverSlice.actions.setCommandLoading({ command }));
try {
reportEvent(Events.TEXT_COMMAND_RUN);
const text = await command.handler(getElementText(element));
await replaceAtCommand({ commandKey, element, text });
dispatch(popoverSlice.actions.setCommandSuccess({ text }));
await replaceAtCommand({ commandKey, query, element, text });
onHide();
if (isMounted()) {
// We're setting success state for Storybook. In practice, the popover will be unmounted via onHide()
dispatch(popoverSlice.actions.setCommandSuccess({ text }));
}
} catch (error) {
dispatch(popoverSlice.actions.setCommandError({ error }));
}
},
[element, commandKey, onHide, dispatch],
[element, commandKey, onHide, dispatch, isMounted],
);

const query = useKeyboardQuery({
element,
commandKey,
// OK to pass handlers directly because hook uses useRef
async onSubmit() {
async onSubmit(query) {
if (selectedCommandRef.current != null) {
await fillAtCursor(selectedCommandRef.current);
await fillAtCursor({ command: selectedCommandRef.current, query });
}
},
onOffset(offset: number) {
Expand All @@ -158,24 +164,33 @@ const CommandPopover: React.FunctionComponent<
dispatch(popoverSlice.actions.search({ commands, query }));
}, [query, commands, dispatch]);

let status = null;

if (state.activeCommand?.state.isFetching) {
status = (
<div role="status" className="status status--fetching">
Running command: {state.activeCommand.command.title}
</div>
);
} else if (state.activeCommand?.state.isError) {
status = (
<div role="status" className="status status--error">
Error running last command
</div>
);
} else if (state.results.length === 0) {
status = (
<div role="status" className="status status--empty">
No matches found
</div>
);
}

return (
<ShadowRoot mode="open">
<Stylesheets href={[stylesUrl]}>
<div role="menu" aria-label="Text command menu">
{state.activeCommand?.state.isFetching && (
<span className="text-info">
Running command: {state.activeCommand.command.title}
</span>
)}
{state.activeCommand?.state.isError && (
<span className="text-danger">
Error running command:{" "}
{truncate(getErrorMessage(state.activeCommand.state.error), {
length: 25,
})}
</span>
)}

<div role="menu" aria-label="Text command menu" className="root">
{status}
<div className="results">
{state.results.map((command) => {
const isSelected = selectedCommand?.shortcut === command.shortcut;
Expand All @@ -188,14 +203,11 @@ const CommandPopover: React.FunctionComponent<
commandKey={commandKey}
query={state.query ?? ""}
onClick={async () => {
await fillAtCursor(command);
await fillAtCursor({ command, query: query ?? "" });
}}
/>
);
})}
{state.results.length === 0 && (
<span className="text-muted">No commands found</span>
)}
</div>
</div>
</Stylesheets>
Expand Down
Loading
Loading