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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CodeEditor: Ensure suggestions only apply to the instance of the editor that registered them #69995

Merged
merged 6 commits into from Jun 14, 2023
Merged

CodeEditor: Ensure suggestions only apply to the instance of the editor that registered them #69995

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b546486
user essentials mob! :trident:
ashharrison90 Jun 13, 2023
84623b2
user essentials mob! :trident:
L-M-K-B Jun 13, 2023
a06aa67
user essentials mob! :trident:
tskarhed Jun 13, 2023
dd5f416
user essentials mob! :trident:
ashharrison90 Jun 13, 2023
489840a
remove duplicate editor from story
ashharrison90 Jun 13, 2023
a60323a
remove suggestions from story
ashharrison90 Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ export const Basic: StoryFn<typeof CodeEditor> = (args) => {
onSave={(text: string) => {
action('code saved')(text);
}}
getSuggestions={() => [{ label: 'foo' }]}
showLineNumbers={args.showLineNumbers}
showMiniMap={args.showMiniMap}
readOnly={args.readOnly}
Expand Down
16 changes: 9 additions & 7 deletions packages/grafana-ui/src/components/Monaco/CodeEditor.tsx
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ type Props = CodeEditorProps & Themeable2;
class UnthemedCodeEditor extends PureComponent<Props> {
completionCancel?: monacoType.IDisposable;
monaco?: Monaco;
modelId?: string;

constructor(props: Props) {
super(props);
Expand Down Expand Up @@ -44,8 +45,8 @@ class UnthemedCodeEditor extends PureComponent<Props> {
return;
}

if (getSuggestions) {
this.completionCancel = registerSuggestions(this.monaco, language, getSuggestions);
if (getSuggestions && this.modelId) {
this.completionCancel = registerSuggestions(this.monaco, language, getSuggestions, this.modelId);
}
}

Expand Down Expand Up @@ -85,20 +86,21 @@ class UnthemedCodeEditor extends PureComponent<Props> {

handleBeforeMount = (monaco: Monaco) => {
this.monaco = monaco;
const { language, getSuggestions, onBeforeEditorMount } = this.props;

if (getSuggestions) {
this.completionCancel = registerSuggestions(monaco, language, getSuggestions);
}
Comment on lines -90 to -92
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the logical difference ebtween doing this onMount instead of beforeMount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i guess it will happen slightly later, but by doing it onMount we get access to the editor instance, whereas before mount we only have access to the global monaco instance

const { onBeforeEditorMount } = this.props;

onBeforeEditorMount?.(monaco);
};

handleOnMount = (editor: MonacoEditorType, monaco: Monaco) => {
const { onChange, onEditorDidMount } = this.props;
const { getSuggestions, language, onChange, onEditorDidMount } = this.props;

this.modelId = editor.getModel()?.id;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't require a URI? 馃槷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, i think because the model is associated with this editor (as opposed to getting it from the global monaco namespace)

this.getEditorValue = () => editor.getValue();

if (getSuggestions && this.modelId) {
this.completionCancel = registerSuggestions(monaco, language, getSuggestions, this.modelId);
}
// Save when pressing Ctrl+S or Cmd+S
editor.onKeyDown((e: monacoType.IKeyboardEvent) => {
if (e.keyCode === monaco.KeyCode.KeyS && (e.ctrlKey || e.metaKey)) {
Expand Down
8 changes: 7 additions & 1 deletion packages/grafana-ui/src/components/Monaco/suggestions.ts
View file
Open in desktop
Expand Up @@ -73,7 +73,8 @@ function mapKinds(monaco: Monaco, sug?: CodeEditorSuggestionItemKind): monacoTyp
export function registerSuggestions(
monaco: Monaco,
language: string,
getSuggestions: CodeEditorSuggestionProvider
getSuggestions: CodeEditorSuggestionProvider,
modelId: string
): monacoType.IDisposable | undefined {
if (!language || !getSuggestions) {
return undefined;
Expand All @@ -82,6 +83,11 @@ export function registerSuggestions(
triggerCharacters: ['$'],

provideCompletionItems: (model, position, context) => {
// only return these suggestions for the specified modelId
// prevents duplicate suggestions when multiple editors are open
if (model.id !== modelId) {
return undefined;
}
const range = {
startLineNumber: position.lineNumber,
endLineNumber: position.lineNumber,
Expand Down