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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Wrap code editor for template selector #21104

Merged
merged 3 commits into from
Jun 17, 2024
Merged

Wrap code editor for template selector #21104

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95c15bd
Wrap code editor for template selector
bramkragten Jun 17, 2024
01e74b5
Change to `linewrap`
bramkragten Jun 17, 2024
8f557c0
Update src/components/ha-selector/ha-selector-template.ts
bramkragten Jun 17, 2024
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
12 changes: 12 additions & 0 deletions src/components/ha-code-editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export class HaCodeEditor extends ReactiveElement {

@property({ type: Boolean }) public readOnly = false;

@property({ type: Boolean }) public linewrap = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a brief comment describing the purpose of the linewrap property.

Adding a comment above this property will improve code readability and maintainability, helping future developers understand its use more quickly.


@property({ type: Boolean, attribute: "autocomplete-entities" })
public autocompleteEntities = false;

Expand Down Expand Up @@ -134,6 +136,13 @@ export class HaCodeEditor extends ReactiveElement {
),
});
}
if (changedProps.has("linewrap")) {
transactions.push({
effects: this._loadedCodeMirror!.linewrapCompartment!.reconfigure(
this.linewrap ? this._loadedCodeMirror!.EditorView.lineWrapping : []
),
});
}
Comment on lines +139 to +145
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the use of non-null assertions is safe here.

- this._loadedCodeMirror!.linewrapCompartment!.reconfigure(
+ this._loadedCodeMirror?.linewrapCompartment?.reconfigure(

The non-null assertion operator (!) is used multiple times, which can lead to runtime errors if this._loadedCodeMirror or linewrapCompartment is null. Consider replacing it with optional chaining (?.) to make the code safer.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (changedProps.has("linewrap")) {
transactions.push({
effects: this._loadedCodeMirror!.linewrapCompartment!.reconfigure(
this.linewrap ? this._loadedCodeMirror!.EditorView.lineWrapping : []
),
});
}
if (changedProps.has("linewrap")) {
transactions.push({
effects: this._loadedCodeMirror?.linewrapCompartment?.reconfigure(
this.linewrap ? this._loadedCodeMirror?.EditorView.lineWrapping : []
),
});
}
Tools
Biome

[error] 141-141: Forbidden non-null assertion. (lint/style/noNonNullAssertion)

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

[error] 141-141: Forbidden non-null assertion. (lint/style/noNonNullAssertion)

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

[error] 142-142: Forbidden non-null assertion. (lint/style/noNonNullAssertion)

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

if (changedProps.has("_value") && this._value !== this.value) {
transactions.push({
changes: {
Expand Down Expand Up @@ -181,6 +190,9 @@ export class HaCodeEditor extends ReactiveElement {
this._loadedCodeMirror.readonlyCompartment.of(
this._loadedCodeMirror.EditorView.editable.of(!this.readOnly)
),
this._loadedCodeMirror.linewrapCompartment.of(
this.linewrap ? this._loadedCodeMirror.EditorView.lineWrapping : []
),
Comment on lines +193 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the use of non-null assertions is safe here.

- this._loadedCodeMirror!.linewrapCompartment!.of(
+ this._loadedCodeMirror?.linewrapCompartment?.of(

As previously noted, using non-null assertions without checks can lead to runtime issues. Optional chaining should be used instead to ensure the code is robust against null values.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this._loadedCodeMirror.linewrapCompartment.of(
this.linewrap ? this._loadedCodeMirror.EditorView.lineWrapping : []
),
this._loadedCodeMirror?.linewrapCompartment?.of(
this.linewrap ? this._loadedCodeMirror.EditorView.lineWrapping : []
),

this._loadedCodeMirror.EditorView.updateListener.of(this._onUpdate),
];

Expand Down
1 change: 1 addition & 0 deletions src/components/ha-selector/ha-selector-template.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export class HaTemplateSelector extends LitElement {
autocomplete-icons
@value-changed=${this._handleChange}
dir="ltr"
wrap
bramkragten marked this conversation as resolved.
Show resolved Hide resolved
></ha-code-editor>
${this.helper
? html`<ha-input-helper-text>${this.helper}</ha-input-helper-text>`
Expand Down
1 change: 1 addition & 0 deletions src/resources/codemirror.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const langs = {

export const langCompartment = new Compartment();
export const readonlyCompartment = new Compartment();
export const linewrapCompartment = new Compartment();

export const tabKeyBindings: KeyBinding[] = [
{ key: "Tab", run: indentMore },
Expand Down
Loading