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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
82 changes: 52 additions & 30 deletions src/contentScript/commandPopover/CommandPopover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,18 @@ import stylesUrl from "./CommandPopover.scss?loadAsUrl";
import {
initialState,
popoverSlice,
type PopoverState,
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 +75,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 All @@ -99,43 +102,78 @@ const ResultItem: React.FunctionComponent<{
);
};

const StatusBar: React.FunctionComponent<{
activeCommand?: PopoverState["activeCommand"];
results: PopoverState["results"];
}> = ({ activeCommand, results }) => {
if (activeCommand?.state.isFetching) {
return (
<div role="status" className="status status--fetching">
Running command: {activeCommand.command.title}
</div>
);
}

if (activeCommand?.state.isError) {
return (
<div role="status" className="status status--error">
Error running last command
</div>
);
}

if (results.length === 0) {
return (
<div role="status" className="status status--empty">
No matches found
</div>
);
}

return null;
};

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 @@ -161,21 +199,8 @@ const CommandPopover: React.FunctionComponent<
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">
<StatusBar {...state} />
<div className="results">
{state.results.map((command) => {
const isSelected = selectedCommand?.shortcut === command.shortcut;
Expand All @@ -188,14 +213,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