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

Use Monaco editor in ConfigMap details #6830

Merged
merged 12 commits into from
Dec 29, 2022
25 changes: 14 additions & 11 deletions src/renderer/components/+config-maps/config-map-details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { autorun, makeObservable, observable } from "mobx";
import { disposeOnUnmount, observer } from "mobx-react";
import { DrawerTitle } from "../drawer";
import type { ShowNotification } from "../notifications";
import { Input } from "../input";
import { Button } from "../button";
import type { KubeObjectDetailsProps } from "../kube-object-details";
import { ConfigMap } from "../../../common/k8s-api/endpoints";
Expand All @@ -21,6 +20,7 @@ import configMapStoreInjectable from "./store.injectable";
import showSuccessNotificationInjectable from "../notifications/show-success-notification.injectable";
import showErrorNotificationInjectable from "../notifications/show-error-notification.injectable";
import loggerInjectable from "../../../common/logger.injectable";
import { MonacoEditor } from "../monaco-editor";

export interface ConfigMapDetailsProps extends KubeObjectDetailsProps<ConfigMap> {
}
Expand Down Expand Up @@ -99,18 +99,21 @@ class NonInjectedConfigMapDetails extends React.Component<ConfigMapDetailsProps
<>
<DrawerTitle>Data</DrawerTitle>
{
data.map(([name, value]) => (
data.map(([name, value = ""]) => (
<div key={name} className="data">
<div className="name">{name}</div>
<div className="flex gaps align-flex-start">
<Input
multiLine
theme="round-black"
className="box grow"
value={value}
onChange={v => this.data.set(name, v)}
/>
</div>
<MonacoEditor
id={`config-map-data-${name}`}
style={{
resize: "vertical",
overflow: "hidden",
border: "1px solid var(--borderFaintColor)",
borderRadius: "4px",
}}
value={value}
onChange={v => this.data.set(name, v)}
setInitialHeight
/>
</div>
))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/

import { getInjectable } from "@ogre-tools/injectable";

const getEditorHeightFromLinesCountInjectable = getInjectable({
Copy link
Contributor

@ixrock ixrock Dec 28, 2022

Choose a reason for hiding this comment

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

So why in general do we need this function/feature to be *.injectable.ts?
Any useful examples of how it would be utilized later in other envs? Or what's purpose in other words?

Copy link
Contributor Author

@aleksfront aleksfront Dec 29, 2022

Choose a reason for hiding this comment

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

It is implemented as injectable to follow de-facto standard and being able to inject it as dependency into MonacoEditor:

export const MonacoEditor = withInjectables<Dependencies, MonacoEditorProps, MonacoEditorRef>(
  React.forwardRef<MonacoEditorRef, MonacoEditorProps & Dependencies>((props, ref) => <NonInjectedMonacoEditor innerRef={ref} {...props} />),
  {
    getProps: (di, props) => ({
      ...props,
      userStore: di.inject(userStoreInjectable),
      activeTheme: di.inject(activeThemeInjectable),
      getEditorHeightFromLinesCount: di.inject(getEditorHeightFromLinesCountInjectable),
    }),
  },
);

Without injectable format it would be used from the shared state (eg. imported module) which is considered as bad practice.

Copy link
Contributor

@ixrock ixrock Dec 29, 2022

Choose a reason for hiding this comment

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

Alright, but why you've decided to control only height from MonacoEditor and this is why it's moved to injectable.ts?
You still didn't answer my question: if you able to control height of the editor, when/where would be reused?
Some examples? Maybe builds for lens-ide or lens-some-build with different initial heights for the editor? This would make sense?

id: "get-editor-height-from-lines-number",

instantiate: () => {
return (linesCount: number) => {
if (typeof linesCount !== "number") {
throw new Error("linesNumber must be a number");
}

if (linesCount < 10) {
return 90;
}

if (linesCount < 20) {
return 180;
}

return 360;
};
},
});

export default getEditorHeightFromLinesCountInjectable;
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright (c) OpenLens Authors. All rights reserved.
* Licensed under MIT License. See LICENSE in root directory for more information.
*/

import { getDiForUnitTesting } from "../../getDiForUnitTesting";
import getEditorHeightFromLinesCountInjectable from "./get-editor-height-from-lines-number.injectable";

describe("get-editor-height-from-lines-number", () => {
let getEditorHeightFromLinesNumber: (linesCount: number) => number;

beforeEach(() => {
const di = getDiForUnitTesting({ doGeneralOverrides: false });

getEditorHeightFromLinesNumber = di.inject(getEditorHeightFromLinesCountInjectable);
});

it("given linesCount is less than 10, when called, returns small number", () => {
const actual = getEditorHeightFromLinesNumber(9);

expect(actual).toBe(90);
});

it("given linesCount is equal to 10, when called, returns medium number", () => {
const actual = getEditorHeightFromLinesNumber(10);

expect(actual).toBe(180);
});

it("given linesCount is greater than 10 and less than 20, when called, returns medium number", () => {
const actual = getEditorHeightFromLinesNumber(19);

expect(actual).toBe(180);
});

it("given linesCount is equal to 20, when called, returns large number", () => {
const actual = getEditorHeightFromLinesNumber(20);

expect(actual).toBe(360);
});

it("given linesCount is greater than 20, when called, returns large number", () => {
const actual = getEditorHeightFromLinesNumber(21);

expect(actual).toBe(360);
});
});
15 changes: 14 additions & 1 deletion src/renderer/components/monaco-editor/monaco-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { LensTheme } from "../../themes/lens-theme";
import { withInjectables } from "@ogre-tools/injectable-react";
import userStoreInjectable from "../../../common/user-store/user-store.injectable";
import activeThemeInjectable from "../../themes/active.injectable";
import getEditorHeightFromLinesCountInjectable from "./get-editor-height-from-lines-number.injectable";

export type MonacoEditorId = string;

Expand All @@ -37,11 +38,13 @@ export interface MonacoEditorProps {
onDidContentSizeChange?(evt: editor.IContentSizeChangedEvent): void;
onModelChange?(model: editor.ITextModel, prev?: editor.ITextModel): void;
innerRef?: React.ForwardedRef<MonacoEditorRef>;
setInitialHeight?: boolean;
}

interface Dependencies {
userStore: UserStore;
activeTheme: IComputedValue<LensTheme>;
getEditorHeightFromLinesCount: (linesCount: number) => number;
}

export function createMonacoUri(id: MonacoEditorId): Uri {
Expand Down Expand Up @@ -288,14 +291,23 @@ class NonInjectedMonacoEditor extends React.Component<MonacoEditorProps & Depend
// avoid excessive validations during typing
validateLazy = debounce(this.validate, 250);

get initialHeight() {
return this.props.getEditorHeightFromLinesCount(this.model.getLineCount());
}

render() {
const { className, style } = this.props;

const css: React.CSSProperties = {
...style,
height: style?.height ?? this.initialHeight,
};

return (
<div
data-test-id="monaco-editor"
className={cssNames(styles.MonacoEditor, className)}
style={style}
style={css}
ref={elem => this.containerElem = elem}
/>
);
Expand All @@ -309,6 +321,7 @@ export const MonacoEditor = withInjectables<Dependencies, MonacoEditorProps, Mon
...props,
userStore: di.inject(userStoreInjectable),
activeTheme: di.inject(activeThemeInjectable),
getEditorHeightFromLinesCount: di.inject(getEditorHeightFromLinesCountInjectable),
}),
},
);