Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

Fix jumpy action buttons in the editor panel #40

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Fix jumpy action buttons in the editor panel #40

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a59c7e0
Fix jumpy action buttons in the editor panel
Andarist Jul 13, 2021
193acbd
Fixed Monaco editor resizing issue
Andarist Jul 14, 2021
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
5 changes: 0 additions & 5 deletions src/EditorPanel.scss
View file
Open in desktop

This file was deleted.

7 changes: 3 additions & 4 deletions src/EditorPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { Button, HStack } from '@chakra-ui/react';
import { Button, HStack, Box } from '@chakra-ui/react';
import { useMachine, useSelector } from '@xstate/react';
import React from 'react';
import { ActorRefFrom, send } from 'xstate';
import { assign } from 'xstate';
import { createMachine, send as sendAction, spawn } from 'xstate';
import { createModel } from 'xstate/lib/model';
import { useClient } from './clientContext';
import './EditorPanel.scss';
import { EditorWithXStateImports } from './EditorWithXStateImports';
import { notifMachine } from './notificationMachine';
import { parseMachines } from './parseMachine';
Expand Down Expand Up @@ -150,7 +149,7 @@ export const EditorPanel: React.FC<{
);

return (
<div data-panel="editor">
<Box height="100%" display="grid" gridTemplateRows="1fr auto">
<EditorWithXStateImports
defaultValue={defaultValue}
readonly={current.matches('compiling')}
Expand Down Expand Up @@ -183,6 +182,6 @@ export const EditorPanel: React.FC<{
{persistText}
</Button>
</HStack>
</div>
</Box>
);
};
81 changes: 44 additions & 37 deletions src/EditorWithXStateImports.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ClassNames } from '@emotion/react';
import Editor, { OnMount } from '@monaco-editor/react';
import { SpinnerWithText } from './SpinnerWithText';

Expand All @@ -12,45 +13,51 @@ export const EditorWithXStateImports = (
props: EditorWithXStateImportsProps,
) => {
return (
<Editor
height="auto"
defaultPath="main.ts"
defaultLanguage="typescript"
defaultValue={props.defaultValue}
theme="vs-dark"
options={{ readOnly: props.readonly }}
loading={<SpinnerWithText text="Preparing the editor" />}
onChange={(text) => {
if (text) {
props.onChange?.(text);
}
}}
onMount={async (editor, monaco) => {
monaco.languages.typescript.typescriptDefaults.setWorkerOptions({
customWorkerPath: `${new URL(
process.env.PUBLIC_URL,
window.location.origin,
)}/ts-worker.js`,
});
monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
...monaco.languages.typescript.typescriptDefaults.getCompilerOptions(),
module: monaco.languages.typescript.ModuleKind.CommonJS,
moduleResolution:
monaco.languages.typescript.ModuleResolutionKind.NodeJs,
strict: true,
});
<ClassNames>
{({ css }) => (
<Editor
wrapperClassName={css`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't wrapperClassName be an actual class name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it is. That's why I've introduced <ClassNames/> here - this is a special component that provides a css that actually injects a class name eagerly to the CSSOM and returns it:
https://github.com/emotion-js/emotion/blob/dcc72d06ace804330fd285a76c8574f3a89001f9/packages/react/src/class-names.js#L113

This is not the same css as the one that can be imported from @emotion/react. Since we are using Chakra right now I've chosen this solution to stick as much as possible to a single styling solution, instead of mixing both Chakra/Emotion and plain CSS/SCSS

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it'd be easier for theming if we go with this solution. in that case, all we need to tweak is the main Chakra theme + some manual work in the canvas

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I admit the API here is confusing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the API is confusing and is one of the main complaints that we receive for Emotion. It is designed like that for a reason though: render prop can actually render something and, by default, during SSR we render <style/> elements inline. This makes SSR of Emotion working out of the box, no special setup etc is required to make it work. We know that not everyone is SSRing but exposing a hook-based API that would not solve all the problems has always seemed less than ideal for us.

Usually, this is not that big of a problem in codebases because it's not as common to need access to the actual class name, it's mainly when integrating with external libraries that accept class names in different props than the actual className prop (which is the case here, we need to pass this to the wrapperClassName prop). And those external components can usually just be isolated in the codebase so we don't have to use <ClassNames/> component too much.

There is a chance that React 18 will bring some new kind of an API, specific to CSS injection so maybe we'll be able to get rid of this in Emotion in the future. Time will tell though.

There is also a chance that we'll be able to remove this in our codebase if this gets merged in: suren-atoyan/monaco-react#269

overflow: hidden;
`}
defaultPath="main.ts"
defaultLanguage="typescript"
defaultValue={props.defaultValue}
theme="vs-dark"
options={{ readOnly: props.readonly }}
loading={<SpinnerWithText text="Preparing the editor" />}
onChange={(text) => {
if (text) {
props.onChange?.(text);
}
}}
onMount={async (editor, monaco) => {
monaco.languages.typescript.typescriptDefaults.setWorkerOptions({
customWorkerPath: `${new URL(
process.env.PUBLIC_URL,
window.location.origin,
)}/ts-worker.js`,
});
monaco.languages.typescript.typescriptDefaults.setCompilerOptions({
...monaco.languages.typescript.typescriptDefaults.getCompilerOptions(),
module: monaco.languages.typescript.ModuleKind.CommonJS,
moduleResolution:
monaco.languages.typescript.ModuleResolutionKind.NodeJs,
strict: true,
});

const indexFile = await fetch(`/xstate.d.ts.txt`).then((res) =>
res.text(),
);
const indexFile = await fetch(`/xstate.d.ts.txt`).then((res) =>
res.text(),
);

monaco.languages.typescript.typescriptDefaults.addExtraLib(
indexFile,
'file:///node_modules/xstate/index.d.ts',
);
monaco.languages.typescript.typescriptDefaults.addExtraLib(
indexFile,
'file:///node_modules/xstate/index.d.ts',
);

props.onMount?.(editor, monaco);
}}
/>
props.onMount?.(editor, monaco);
}}
/>
)}
</ClassNames>
);
};