From 6004dda79f4b7ca9b58e3650cad0be9f9cb8021c Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 25 Jul 2023 13:29:49 +0200 Subject: [PATCH 01/27] Create overlay layer fopr marking --- .../annotations/code_listing_row.ts | 98 +------------- .../components/annotations/marking_layer.ts | 122 ++++++++++++++++++ app/assets/javascripts/state/Submissions.ts | 14 +- .../components/code_listing.css.scss | 11 ++ 4 files changed, 149 insertions(+), 96 deletions(-) create mode 100644 app/assets/javascripts/components/annotations/marking_layer.ts diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index 622f0150d2..3b52a49f51 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -9,14 +9,12 @@ import { i18nMixin } from "components/meta/i18n_mixin"; import { initTooltips } from "util.js"; import { PropertyValues } from "@lit/reactive-element"; import { userState } from "state/Users"; -import { AnnotationData, annotationState, compareAnnotationOrders } from "state/Annotations"; -import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations"; -import { wrapRangesInHtml, range, wrapRangesInHtmlCached } from "mark"; -import { SelectedRange, UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; -import { AnnotationMarker } from "components/annotations/annotation_marker"; +import { annotationState } from "state/Annotations"; +import { userAnnotationState } from "state/UserAnnotations"; import "components/annotations/selection_marker"; import "components/annotations/create_annotation_button"; import { triggerSelectionStart } from "components/annotations/select"; +import "components/annotations/marking_layer"; /** * This component represents a row in the code listing. @@ -35,60 +33,6 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) { @property({ type: String, attribute: "rendered-code" }) renderedCode: string; - - /** - * Calculates the range of the code that is covered by the given annotation. - * If the annotation spans multiple lines, the range will be the whole line unless this is the first or last line. - * In that case, the range will be the part of the line that is covered by the annotation. - * @param annotation The annotation to calculate the range for. - */ - getRangeFromAnnotation(annotation: AnnotationData | SelectedRange): range { - const isMachineAnnotation = ["error", "warning", "info"].includes((annotation as AnnotationData).type); - const rowsLength = annotation.rows ?? 1; - let lastRow = annotation.row ? annotation.row + rowsLength : 0; - let firstRow = annotation.row ? annotation.row + 1 : 0; - - if (!isMachineAnnotation) { - // rows on user annotations are 1-based, so we need to subtract 1 - firstRow -= 1; - lastRow -= 1; - } - - let start = 0; - if (this.row === firstRow) { - start = annotation.column || 0; - } - - let length = Infinity; - if (this.row === lastRow) { - if (annotation.column !== undefined && annotation.column !== null) { - const defaultLength = isMachineAnnotation ? 0 : Infinity; - length = annotation.columns || defaultLength; - } - } - - return { start: start, length: length, data: annotation }; - } - - get wrappedCode(): string { - const annotationsToMark = [...this.userAnnotationsToMark, ...this.machineAnnotationsToMark].sort(compareAnnotationOrders); - const codeToMark = this.renderedCode; - let annotationsMarked = wrapRangesInHtmlCached( - codeToMark, - annotationsToMark.map(a => this.getRangeFromAnnotation(a)), - "d-annotation-marker", - (node: AnnotationMarker, range) => { - // these nodes will be recompiled to html, so we need to store the data in a json string - const annotations = JSON.parse(node.getAttribute("annotations")) || []; - annotations.push(range.data); - node.setAttribute("annotations", JSON.stringify(annotations)); - }); - if ( userAnnotationState.formShown && this.shouldMarkSelection ) { - annotationsMarked = wrapRangesInHtml(annotationsMarked, [this.getRangeFromAnnotation(userAnnotationState.selectedRange)], "d-selection-marker"); - } - return annotationsMarked; - } - firstUpdated(_changedProperties: PropertyValues): void { super.firstUpdated(_changedProperties); initTooltips(this); @@ -99,20 +43,6 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) { return userState.hasPermission("annotation.create"); } - get machineAnnotationsToMark(): MachineAnnotationData[] { - return machineAnnotationState.byMarkedLine.get(this.row) || []; - } - - get userAnnotationsToMark(): UserAnnotationData[] { - return userAnnotationState.rootIdsByMarkedLine.get(this.row)?.map(i => userAnnotationState.byId.get(i)) || []; - } - - get shouldMarkSelection(): boolean { - return userAnnotationState.selectedRange && - userAnnotationState.selectedRange.row <= this.row && - userAnnotationState.selectedRange.row + (userAnnotationState.selectedRange.rows ?? 1) > this.row; - } - get formShown(): boolean { const range = userAnnotationState.selectedRange; return userAnnotationState.formShown && range && range.row + range.rows - 1 === this.row; @@ -123,20 +53,6 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) { userAnnotationState.selectedRange = undefined; } - get fullLineAnnotations(): UserAnnotationData[] { - return this.userAnnotationsToMark - .filter(a => !a.column&& !a.columns) - .sort(compareAnnotationOrders); - } - - get hasFullLineSelection(): boolean { - return this.shouldMarkSelection && !userAnnotationState.selectedRange.column && !userAnnotationState.selectedRange.columns; - } - - get codeLineClass(): string { - return this.hasFullLineSelection ? `code-line-${annotationState.isQuestionMode ? "question" : "annotation"}` : ""; - } - dragEnter(e: DragEvent): void { if (userAnnotationState.dragStartRow === null) { return; @@ -164,13 +80,7 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
${this.row}
- ${this.fullLineAnnotations.length > 0 ? html` - -
${unsafeHTML(this.wrappedCode)}
-
- ` : html` -
${unsafeHTML(this.wrappedCode)}
- `} +
${unsafeHTML(this.renderedCode)}
this.closeForm()} diff --git a/app/assets/javascripts/components/annotations/marking_layer.ts b/app/assets/javascripts/components/annotations/marking_layer.ts new file mode 100644 index 0000000000..4808cad06e --- /dev/null +++ b/app/assets/javascripts/components/annotations/marking_layer.ts @@ -0,0 +1,122 @@ +import { ShadowlessLitElement } from "components/meta/shadowless_lit_element"; +import { customElement, property } from "lit/decorators.js"; +import { html, TemplateResult } from "lit"; +import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations"; +import { SelectedRange, UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; +import { AnnotationData } from "state/Annotations"; +import { submissionState } from "state/Submissions"; + +declare type range = { + start: number; + length: number; + annotations: (AnnotationData | SelectedRange)[]; +}; + +function numberArrayEquals(a: number[], b: number[]): boolean { + return a.length === b.length && a.every((v, i) => v === b[i]); +} + +@customElement("d-marking-layer") +export class MarkingLayer extends ShadowlessLitElement { + @property({ type: Number }) + row: number; + + get code(): string { + return submissionState.codeByLine[this.row - 1]; + } + + get codeLength(): number { + return this.code.length; + } + + get machineAnnotationsToMark(): MachineAnnotationData[] { + return machineAnnotationState.byMarkedLine.get(this.row) || []; + } + + get userAnnotationsToMark(): UserAnnotationData[] { + return userAnnotationState.rootIdsByMarkedLine.get(this.row)?.map(i => userAnnotationState.byId.get(i)) || []; + } + + get shouldMarkSelection(): boolean { + return userAnnotationState.selectedRange && + userAnnotationState.selectedRange.row <= this.row && + userAnnotationState.selectedRange.row + (userAnnotationState.selectedRange.rows ?? 1) > this.row; + } + + /** + * Calculates the range of the code that is covered by the given annotation. + * If the annotation spans multiple lines, the range will be the whole line unless this is the first or last line. + * In that case, the range will be the part of the line that is covered by the annotation. + * @param annotation The annotation to calculate the range for. + */ + getRangeFromAnnotation(annotation: AnnotationData | SelectedRange, index: number): { start: number, length: number, index: number } { + const isMachineAnnotation = ["error", "warning", "info"].includes((annotation as AnnotationData).type); + const rowsLength = annotation.rows ?? 1; + let lastRow = annotation.row ? annotation.row + rowsLength : 0; + let firstRow = annotation.row ? annotation.row + 1 : 0; + + if (!isMachineAnnotation) { + // rows on user annotations are 1-based, so we need to subtract 1 + firstRow -= 1; + lastRow -= 1; + } + + let start = 0; + if (this.row === firstRow) { + start = annotation.column || 0; + } + + let length = this.codeLength - start; + if (this.row === lastRow) { + if (annotation.column !== undefined && annotation.column !== null) { + const defaultLength = isMachineAnnotation ? 0 : this.codeLength - start; + length = annotation.columns || defaultLength; + } + } + + return { start: start, length: length, index: index }; + } + + mergeRanges(ranges: { start: number, length: number, index: number }[]): { start: number, length: number, indexes: number[] }[] { + console.log("merge_ranges", this.row, ranges); + const annotationsByPosition: number[][] = new Array(this.codeLength).fill(null).map(() => []); + for (const range of ranges) { + for (let i = range.start; i < range.start + range.length; i++) { + annotationsByPosition[i].push(range.index); + } + } + const rangesToReturn = []; + let i = 0; + while (i < this.codeLength) { + let j = 1; + while (i + j < this.codeLength && numberArrayEquals(annotationsByPosition[i + j], annotationsByPosition[i])) { + j++; + } + rangesToReturn.push({ start: i, length: j, indexes: annotationsByPosition[i] }); + i += j; + } + return rangesToReturn; + } + + get ranges(): range[] { + const toMark: (AnnotationData | SelectedRange)[] = [...this.machineAnnotationsToMark, ...this.userAnnotationsToMark]; + if (this.shouldMarkSelection) { + toMark.push(userAnnotationState.selectedRange); + } + console.log("to_mark", this.row, toMark); + // We use indexes to simplify the equality check in mergeRanges + const ranges = toMark.map((annotation, index) => this.getRangeFromAnnotation(annotation, index)); + const mergedRanges = this.mergeRanges(ranges); + return mergedRanges.map(range => { + const annotations = range.indexes.map(i => toMark[i]); + return { start: range.start, length: range.length, annotations: annotations }; + }); + } + + render(): TemplateResult { + console.log(this.row, this.ranges); + return html`${this.ranges.map(range => range.annotations.length ? + html`${this.code.substring(range.start, range.start + range.length)}` : + html`${this.code.substring(range.start, range.start + range.length)}`)}`; + } +} diff --git a/app/assets/javascripts/state/Submissions.ts b/app/assets/javascripts/state/Submissions.ts index 8d3e436f26..d104809b55 100644 --- a/app/assets/javascripts/state/Submissions.ts +++ b/app/assets/javascripts/state/Submissions.ts @@ -2,8 +2,18 @@ import { State } from "state/state_system/State"; import { stateProperty } from "state/state_system/StateProperty"; class SubmissionState extends State { - @stateProperty id; - @stateProperty code; + @stateProperty id: number; + @stateProperty _code: string; + @stateProperty codeByLine: string[]; + + set code(code: string) { + this._code = code; + this.codeByLine = code.split("\n"); + } + + get code(): string { + return this._code; + } } export const submissionState = new SubmissionState(); diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index 778befa7f4..04d50ee838 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -292,6 +292,17 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 overflow: visible; display: inline-block; padding-top: 1px; + position: relative; + + d-marking-layer { + position: absolute; + top: 2px; + left: 4px; + width: 100%; + height: 100%; + pointer-events: none; + color: transparent; + } } // This is a hack to extend the drop target more to the left From 06aafd705a8f963ea74e09508b157fed2fca2117 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 25 Jul 2023 14:35:21 +0200 Subject: [PATCH 02/27] Fix color and continous lines --- .../components/annotations/annotation_marker.ts | 5 ++++- app/assets/stylesheets/components/code_listing.css.scss | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/components/annotations/annotation_marker.ts b/app/assets/javascripts/components/annotations/annotation_marker.ts index 293f5445c1..0213223591 100644 --- a/app/assets/javascripts/components/annotations/annotation_marker.ts +++ b/app/assets/javascripts/components/annotations/annotation_marker.ts @@ -33,7 +33,10 @@ export class AnnotationMarker extends LitElement { static getStyle(annotation: AnnotationData): string { if (["error", "warning", "info"].includes(annotation.type)) { - return `text-decoration: wavy underline ${AnnotationMarker.colors[annotation.type]} 1px;`; + return ` + text-decoration: wavy underline ${AnnotationMarker.colors[annotation.type]} 1px; + text-decoration-skip-ink: none; + `; } else { return ` background: ${AnnotationMarker.colors[annotation.type]}; diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index 04d50ee838..522f083675 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -8,10 +8,10 @@ $annotation-user: $success; $annotation-error: $danger; $annotation-warning: $warning; $annotation-info: $info; -$annotation-question-background: color.mix($annotation-question, $background, 8%); -$annotation-user-background: color.mix($annotation-user, $background, 8%); -$annotation-question-background-intense: color.mix($annotation-question, $background, 20%); -$annotation-user-background-intense: color.mix($annotation-user, $background, 20%); +$annotation-question-background: rgba(color.red($annotation-question), color.green($annotation-question), color.blue($annotation-question), 0.08); +$annotation-user-background: rgba(color.red($annotation-user), color.green($annotation-user), color.blue($annotation-user), 0.08); +$annotation-question-background-intense: rgba(color.red($annotation-question), color.green($annotation-question), color.blue($annotation-question), 0.2); +$annotation-user-background-intense: rgba(color.red($annotation-user), color.green($annotation-user), color.blue($annotation-user), 0.2); .code-table { &.selection-color-question { From ff19abd11eb4a9c24bc5328d0c878993f2c6afa5 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 25 Jul 2023 15:26:14 +0200 Subject: [PATCH 03/27] Fix zero length ranges --- .../components/annotations/marking_layer.ts | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/components/annotations/marking_layer.ts b/app/assets/javascripts/components/annotations/marking_layer.ts index 4808cad06e..9494ea1534 100644 --- a/app/assets/javascripts/components/annotations/marking_layer.ts +++ b/app/assets/javascripts/components/annotations/marking_layer.ts @@ -78,23 +78,40 @@ export class MarkingLayer extends ShadowlessLitElement { } mergeRanges(ranges: { start: number, length: number, index: number }[]): { start: number, length: number, indexes: number[] }[] { - console.log("merge_ranges", this.row, ranges); const annotationsByPosition: number[][] = new Array(this.codeLength).fill(null).map(() => []); for (const range of ranges) { for (let i = range.start; i < range.start + range.length; i++) { annotationsByPosition[i].push(range.index); } } + + const zeroLengthRanges = ranges.filter(range => range.length === 0); + const zeroLengthIndexesByPosition: number[][] = new Array(this.codeLength+1).fill(null).map(() => []); + for (const range of zeroLengthRanges) { + if (range.start >= this.codeLength) { + console.log(this.row); + } + zeroLengthIndexesByPosition[range.start].push(range.index); + } + const rangesToReturn = []; let i = 0; while (i < this.codeLength) { + if (zeroLengthIndexesByPosition[i].length) { + rangesToReturn.push({ start: i, length: 0, indexes: zeroLengthIndexesByPosition[i] }); + } let j = 1; - while (i + j < this.codeLength && numberArrayEquals(annotationsByPosition[i + j], annotationsByPosition[i])) { + while (i + j < this.codeLength && numberArrayEquals(annotationsByPosition[i + j], annotationsByPosition[i]) && !zeroLengthIndexesByPosition[i + j].length) { j++; } rangesToReturn.push({ start: i, length: j, indexes: annotationsByPosition[i] }); i += j; } + + // Add the zero length ranges at the end of the line + if (zeroLengthIndexesByPosition[this.codeLength].length) { + rangesToReturn.push({ start: this.codeLength, length: 0, indexes: zeroLengthIndexesByPosition[this.codeLength] }); + } return rangesToReturn; } @@ -103,7 +120,6 @@ export class MarkingLayer extends ShadowlessLitElement { if (this.shouldMarkSelection) { toMark.push(userAnnotationState.selectedRange); } - console.log("to_mark", this.row, toMark); // We use indexes to simplify the equality check in mergeRanges const ranges = toMark.map((annotation, index) => this.getRangeFromAnnotation(annotation, index)); const mergedRanges = this.mergeRanges(ranges); @@ -114,7 +130,6 @@ export class MarkingLayer extends ShadowlessLitElement { } render(): TemplateResult { - console.log(this.row, this.ranges); return html`${this.ranges.map(range => range.annotations.length ? html`${this.code.substring(range.start, range.start + range.length)}` : html`${this.code.substring(range.start, range.start + range.length)}`)}`; From 98a1f4317192825a2078239d08e06b52245c566e Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 25 Jul 2023 15:26:34 +0200 Subject: [PATCH 04/27] remove logging --- app/assets/javascripts/components/annotations/marking_layer.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/assets/javascripts/components/annotations/marking_layer.ts b/app/assets/javascripts/components/annotations/marking_layer.ts index 9494ea1534..cffe1244b2 100644 --- a/app/assets/javascripts/components/annotations/marking_layer.ts +++ b/app/assets/javascripts/components/annotations/marking_layer.ts @@ -88,9 +88,6 @@ export class MarkingLayer extends ShadowlessLitElement { const zeroLengthRanges = ranges.filter(range => range.length === 0); const zeroLengthIndexesByPosition: number[][] = new Array(this.codeLength+1).fill(null).map(() => []); for (const range of zeroLengthRanges) { - if (range.start >= this.codeLength) { - console.log(this.row); - } zeroLengthIndexesByPosition[range.start].push(range.index); } From 72101b655eadeab04fed48cea6c39092d12b5405 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jul 2023 10:52:05 +0200 Subject: [PATCH 05/27] Split code in three layers --- .../annotations/annotation_marker.ts | 27 ++++++----- .../annotations/code_listing_row.ts | 6 ++- .../components/annotations/marking_layer.ts | 24 ++++++++-- .../components/code_listing.css.scss | 47 ++++++++++++------- 4 files changed, 69 insertions(+), 35 deletions(-) diff --git a/app/assets/javascripts/components/annotations/annotation_marker.ts b/app/assets/javascripts/components/annotations/annotation_marker.ts index 0213223591..03d5dfe0aa 100644 --- a/app/assets/javascripts/components/annotations/annotation_marker.ts +++ b/app/assets/javascripts/components/annotations/annotation_marker.ts @@ -17,6 +17,8 @@ const setInstancesDelayer = createDelayer(); export class AnnotationMarker extends LitElement { @property({ type: Array }) annotations: AnnotationData[]; + @property({ type: String }) + type: "background" | "tooltip" = "background"; state = new StateController(this); @@ -40,10 +42,6 @@ export class AnnotationMarker extends LitElement { } else { return ` background: ${AnnotationMarker.colors[annotation.type]}; - padding-top: 2px; - padding-bottom: 2px; - margin-top: -2px; - margin-bottom: -2px; `; } } @@ -118,7 +116,7 @@ export class AnnotationMarker extends LitElement { get machineAnnotationMarkerSVG(): TemplateResult | undefined { const firstMachineAnnotation = this.sortedAnnotations.find(a => !isUserAnnotation(a)); const size = 14; - return firstMachineAnnotation && html` + return firstMachineAnnotation && html` `; } @@ -128,13 +126,16 @@ export class AnnotationMarker extends LitElement { } render(): TemplateResult { - this.renderTooltip(); - - return html`${this.machineAnnotationMarkerSVG}`; + if (this.type === "tooltip") { + this.renderTooltip(); + return html``; + } else { + return html`${this.machineAnnotationMarkerSVG}`; + } } } diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index 3b52a49f51..b425718d95 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -80,7 +80,11 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
${this.row}
-
${unsafeHTML(this.renderedCode)}
+
+ +
${unsafeHTML(this.renderedCode)}
+ +
this.closeForm()} diff --git a/app/assets/javascripts/components/annotations/marking_layer.ts b/app/assets/javascripts/components/annotations/marking_layer.ts index cffe1244b2..12af5e8b61 100644 --- a/app/assets/javascripts/components/annotations/marking_layer.ts +++ b/app/assets/javascripts/components/annotations/marking_layer.ts @@ -3,7 +3,7 @@ import { customElement, property } from "lit/decorators.js"; import { html, TemplateResult } from "lit"; import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations"; import { SelectedRange, UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; -import { AnnotationData } from "state/Annotations"; +import { AnnotationData, compareAnnotationOrders } from "state/Annotations"; import { submissionState } from "state/Submissions"; declare type range = { @@ -20,6 +20,8 @@ function numberArrayEquals(a: number[], b: number[]): boolean { export class MarkingLayer extends ShadowlessLitElement { @property({ type: Number }) row: number; + @property({ type: String }) + type: "background" | "tooltip" = "background"; get code(): string { return submissionState.codeByLine[this.row - 1]; @@ -43,6 +45,12 @@ export class MarkingLayer extends ShadowlessLitElement { userAnnotationState.selectedRange.row + (userAnnotationState.selectedRange.rows ?? 1) > this.row; } + get fullLineAnnotations(): UserAnnotationData[] { + return this.userAnnotationsToMark + .filter(a => !a.column && !a.columns) + .sort(compareAnnotationOrders); + } + /** * Calculates the range of the code that is covered by the given annotation. * If the annotation spans multiple lines, the range will be the whole line unless this is the first or last line. @@ -126,9 +134,17 @@ export class MarkingLayer extends ShadowlessLitElement { }); } + get codeLineClass(): string { + return this.type === "background" ? "code-line background-layer" : "code-line tooltip-layer"; + } + render(): TemplateResult { - return html`${this.ranges.map(range => range.annotations.length ? - html`${this.code.substring(range.start, range.start + range.length)}` : - html`${this.code.substring(range.start, range.start + range.length)}`)}`; + return html` +
${
+    this.ranges.map(range => range.annotations.length ?
+        html`${this.code.substring(range.start, range.start + range.length)}` :
+        html`${this.code.substring(range.start, range.start + range.length)}`)
+}
+
`; } } diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index 522f083675..4429c1dfca 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -8,10 +8,10 @@ $annotation-user: $success; $annotation-error: $danger; $annotation-warning: $warning; $annotation-info: $info; -$annotation-question-background: rgba(color.red($annotation-question), color.green($annotation-question), color.blue($annotation-question), 0.08); -$annotation-user-background: rgba(color.red($annotation-user), color.green($annotation-user), color.blue($annotation-user), 0.08); -$annotation-question-background-intense: rgba(color.red($annotation-question), color.green($annotation-question), color.blue($annotation-question), 0.2); -$annotation-user-background-intense: rgba(color.red($annotation-user), color.green($annotation-user), color.blue($annotation-user), 0.2); +$annotation-question-background: color.mix($annotation-question, $background, 8%); +$annotation-user-background: color.mix($annotation-user, $background, 8%); +$annotation-question-background-intense: color.mix($annotation-question, $background, 20%); +$annotation-user-background-intense: color.mix($annotation-user, $background, 20%); .code-table { &.selection-color-question { @@ -248,10 +248,6 @@ $annotation-user-background-intense: rgba(color.red($annotation-user), color.gre white-space: pre-wrap; word-break: break-word; min-height: 20px; - margin-bottom: -3px; - padding-bottom: 3px; - margin-top: -2px; - padding-top: 2px; } } @@ -288,21 +284,38 @@ $annotation-user-background-intense: rgba(color.red($annotation-user), color.gre d-code-listing-row { display: contents; - pre.code-line { - overflow: visible; - display: inline-block; - padding-top: 1px; + .code-layers { position: relative; + width: 100%; + height: 100%; - d-marking-layer { - position: absolute; - top: 2px; - left: 4px; + .background-layer { width: 100%; height: 100%; - pointer-events: none; color: transparent; } + + .text-layer { + position: absolute; + width: 100%; + height: 100%; + top: 0; + background: transparent; + } + + .tooltip-layer { + position: absolute; + width: 100%; + height: 100%; + top: 0; + opacity: 0; + } + } + + pre.code-line { + overflow: visible; + display: inline-block; + padding-top: 1px; } // This is a hack to extend the drop target more to the left From b567abe1c5182aa4677f616f3f8e943b82f8024d Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jul 2023 14:23:50 +0200 Subject: [PATCH 06/27] Split marking and tooltip layer --- .../annotations/annotation_marker.ts | 100 ++---------------- .../annotations/annotation_tooltip.ts | 94 ++++++++++++++++ .../annotations/code_listing_row.ts | 1 - .../components/annotations/marking_layer.ts | 27 +++-- 4 files changed, 125 insertions(+), 97 deletions(-) create mode 100644 app/assets/javascripts/components/annotations/annotation_tooltip.ts diff --git a/app/assets/javascripts/components/annotations/annotation_marker.ts b/app/assets/javascripts/components/annotations/annotation_marker.ts index 03d5dfe0aa..fa31c95e12 100644 --- a/app/assets/javascripts/components/annotations/annotation_marker.ts +++ b/app/assets/javascripts/components/annotations/annotation_marker.ts @@ -1,15 +1,10 @@ import { customElement, property } from "lit/decorators.js"; -import { render, html, LitElement, TemplateResult } from "lit"; -import tippy, { Instance as Tippy, createSingleton } from "tippy.js"; -import { AnnotationData, annotationState, compareAnnotationOrders, isUserAnnotation } from "state/Annotations"; -import { StateController } from "state/state_system/StateController"; -import { createDelayer } from "util.js"; - -const setInstancesDelayer = createDelayer(); +import { html, LitElement, TemplateResult } from "lit"; +import { AnnotationData, compareAnnotationOrders, isUserAnnotation } from "state/Annotations"; /** - * A marker that styles the slotted content and shows a tooltip with annotations. + * A marker that styles the slotted content based on the relevant annotations. * - * @prop {AnnotationData[]} annotations The annotations to show in the tooltip. + * @prop {AnnotationData[]} annotations The annotations to use for styling. * * @element d-annotation-marker */ @@ -17,11 +12,6 @@ const setInstancesDelayer = createDelayer(); export class AnnotationMarker extends LitElement { @property({ type: Array }) annotations: AnnotationData[]; - @property({ type: String }) - type: "background" | "tooltip" = "background"; - - state = new StateController(this); - static colors = { "error": "var(--error-color, red)", @@ -46,69 +36,6 @@ export class AnnotationMarker extends LitElement { } } - static tippyInstances: Tippy[] = []; - // Using a singleton to avoid multiple tooltips being open at the same time. - static tippySingleton = createSingleton([], { - placement: "bottom-start", - interactive: true, - interactiveDebounce: 25, - delay: [500, 25], - offset: [-10, -2], - // This transition fixes a bug where overlap with the previous tooltip was taken into account when positioning - moveTransition: "transform 0.001s ease-out", - appendTo: () => document.querySelector(".code-table"), - }); - static updateSingletonInstances(): void { - setInstancesDelayer(() => this.tippySingleton.setInstances(this.tippyInstances), 100); - } - static registerTippyInstance(instance: Tippy): void { - this.tippyInstances.push(instance); - this.updateSingletonInstances(); - } - static unregisterTippyInstance(instance: Tippy): void { - this.tippyInstances = this.tippyInstances.filter(i => i !== instance); - this.updateSingletonInstances(); - } - - // Annotations that are displayed inline should show up as tooltips. - get hiddenAnnotations(): AnnotationData[] { - return this.annotations.filter(a => !annotationState.isVisible(a)).sort(compareAnnotationOrders); - } - - tippyInstance: Tippy; - - renderTooltip(): void { - if (this.tippyInstance) { - AnnotationMarker.unregisterTippyInstance(this.tippyInstance); - this.tippyInstance.destroy(); - this.tippyInstance = undefined; - } - - if (this.hiddenAnnotations.length === 0) { - return; - } - - const tooltip = document.createElement("div"); - tooltip.classList.add("marker-tooltip"); - render(this.hiddenAnnotations.map(a => isUserAnnotation(a) ? - html`` : - html``), tooltip); - - this.tippyInstance = tippy(this, { - content: tooltip, - }); - AnnotationMarker.registerTippyInstance(this.tippyInstance); - } - - disconnectedCallback(): void { - super.disconnectedCallback(); - if (this.tippyInstance) { - AnnotationMarker.unregisterTippyInstance(this.tippyInstance); - this.tippyInstance.destroy(); - this.tippyInstance = undefined; - } - } - get sortedAnnotations(): AnnotationData[] { return this.annotations.sort( compareAnnotationOrders ); } @@ -116,7 +43,7 @@ export class AnnotationMarker extends LitElement { get machineAnnotationMarkerSVG(): TemplateResult | undefined { const firstMachineAnnotation = this.sortedAnnotations.find(a => !isUserAnnotation(a)); const size = 14; - return firstMachineAnnotation && html` + return firstMachineAnnotation && html` `; } @@ -126,16 +53,11 @@ export class AnnotationMarker extends LitElement { } render(): TemplateResult { - if (this.type === "tooltip") { - this.renderTooltip(); - return html``; - } else { - return html`${this.machineAnnotationMarkerSVG}`; - } + return html`${this.machineAnnotationMarkerSVG}`; } } diff --git a/app/assets/javascripts/components/annotations/annotation_tooltip.ts b/app/assets/javascripts/components/annotations/annotation_tooltip.ts new file mode 100644 index 0000000000..3211e27038 --- /dev/null +++ b/app/assets/javascripts/components/annotations/annotation_tooltip.ts @@ -0,0 +1,94 @@ +import { customElement, property } from "lit/decorators.js"; +import { render, html, LitElement, TemplateResult, css } from "lit"; +import tippy, { Instance as Tippy, createSingleton } from "tippy.js"; +import { AnnotationData, annotationState, compareAnnotationOrders, isUserAnnotation } from "state/Annotations"; +import { StateController } from "state/state_system/StateController"; +import { createDelayer } from "util.js"; + +const setInstancesDelayer = createDelayer(); +/** + * Adds tooltips with annotations to slotted content + * + * @prop {AnnotationData[]} annotations The annotations to show in the tooltip. + * + * @element d-annotation-tooltip + */ +@customElement("d-annotation-tooltip") +export class AnnotationTooltip extends LitElement { + @property({ type: Array }) + annotations: AnnotationData[]; + + static styles = css`:host { position: relative; }`; + + state = new StateController(this); + + static tippyInstances: Tippy[] = []; + // Using a singleton to avoid multiple tooltips being open at the same time. + static tippySingleton = createSingleton([], { + placement: "bottom-start", + interactive: true, + interactiveDebounce: 25, + delay: [500, 25], + offset: [-10, 0], + // This transition fixes a bug where overlap with the previous tooltip was taken into account when positioning + moveTransition: "transform 0.001s ease-out", + appendTo: () => document.querySelector(".code-table"), + }); + static updateSingletonInstances(): void { + setInstancesDelayer(() => this.tippySingleton.setInstances(this.tippyInstances), 100); + } + static registerTippyInstance(instance: Tippy): void { + this.tippyInstances.push(instance); + this.updateSingletonInstances(); + } + static unregisterTippyInstance(instance: Tippy): void { + this.tippyInstances = this.tippyInstances.filter(i => i !== instance); + this.updateSingletonInstances(); + } + + // Annotations that are displayed inline should show up as tooltips. + get hiddenAnnotations(): AnnotationData[] { + return this.annotations.filter(a => !annotationState.isVisible(a)).sort(compareAnnotationOrders); + } + + tippyInstance: Tippy; + + renderTooltip(): void { + if (this.tippyInstance) { + AnnotationTooltip.unregisterTippyInstance(this.tippyInstance); + this.tippyInstance.destroy(); + this.tippyInstance = undefined; + } + + if (this.hiddenAnnotations.length === 0) { + return; + } + + const tooltip = document.createElement("div"); + tooltip.classList.add("marker-tooltip"); + render(this.hiddenAnnotations.map(a => isUserAnnotation(a) ? + html`` : + html``), tooltip); + + this.tippyInstance = tippy(this, { + content: tooltip, + }); + AnnotationTooltip.registerTippyInstance(this.tippyInstance); + } + + disconnectedCallback(): void { + super.disconnectedCallback(); + if (this.tippyInstance) { + AnnotationTooltip.unregisterTippyInstance(this.tippyInstance); + this.tippyInstance.destroy(); + this.tippyInstance = undefined; + } + } + + render(): TemplateResult { + this.renderTooltip(); + const size = 14; + return html` + `; + } +} diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index b425718d95..c5cd005314 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -3,7 +3,6 @@ import { customElement, property } from "lit/decorators.js"; import { html, TemplateResult } from "lit"; import { unsafeHTML } from "lit/directives/unsafe-html.js"; import "components/annotations/annotations_cell"; -import "components/annotations/annotation_marker"; import "components/annotations/hidden_annotations_dot"; import { i18nMixin } from "components/meta/i18n_mixin"; import { initTooltips } from "util.js"; diff --git a/app/assets/javascripts/components/annotations/marking_layer.ts b/app/assets/javascripts/components/annotations/marking_layer.ts index 12af5e8b61..d9031e2961 100644 --- a/app/assets/javascripts/components/annotations/marking_layer.ts +++ b/app/assets/javascripts/components/annotations/marking_layer.ts @@ -5,6 +5,8 @@ import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnno import { SelectedRange, UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; import { AnnotationData, compareAnnotationOrders } from "state/Annotations"; import { submissionState } from "state/Submissions"; +import "components/annotations/annotation_marker"; +import "components/annotations/annotation_tooltip"; declare type range = { start: number; @@ -139,12 +141,23 @@ export class MarkingLayer extends ShadowlessLitElement { } render(): TemplateResult { - return html` -
${
-    this.ranges.map(range => range.annotations.length ?
-        html`${this.code.substring(range.start, range.start + range.length)}` :
-        html`${this.code.substring(range.start, range.start + range.length)}`)
-}
- `; + const markedCode = this.ranges.map(range => { + const substring = this.code.substring(range.start, range.start + range.length); + if (!range.annotations.length) { + return substring; + } + return this.type === "background" ? + html`${substring}` : + html`${substring}`; + }); + + if (this.type === "background") { + return html` + +
${markedCode}
+
`; + } else { + return html`
${markedCode}
`; + } } } From e4025ec306fa4c425794c5db5c60be69039a1f3d Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jul 2023 14:33:12 +0200 Subject: [PATCH 07/27] simplify code --- .../{marking_layer.ts => code_layers.ts} | 43 ++++++++++--------- .../annotations/code_listing_row.ts | 9 +--- 2 files changed, 24 insertions(+), 28 deletions(-) rename app/assets/javascripts/components/annotations/{marking_layer.ts => code_layers.ts} (84%) diff --git a/app/assets/javascripts/components/annotations/marking_layer.ts b/app/assets/javascripts/components/annotations/code_layers.ts similarity index 84% rename from app/assets/javascripts/components/annotations/marking_layer.ts rename to app/assets/javascripts/components/annotations/code_layers.ts index d9031e2961..8984495661 100644 --- a/app/assets/javascripts/components/annotations/marking_layer.ts +++ b/app/assets/javascripts/components/annotations/code_layers.ts @@ -7,6 +7,7 @@ import { AnnotationData, compareAnnotationOrders } from "state/Annotations"; import { submissionState } from "state/Submissions"; import "components/annotations/annotation_marker"; import "components/annotations/annotation_tooltip"; +import { unsafeHTML } from "lit/directives/unsafe-html.js"; declare type range = { start: number; @@ -18,12 +19,12 @@ function numberArrayEquals(a: number[], b: number[]): boolean { return a.length === b.length && a.every((v, i) => v === b[i]); } -@customElement("d-marking-layer") -export class MarkingLayer extends ShadowlessLitElement { +@customElement("d-code-layers") +export class CodeLayers extends ShadowlessLitElement { @property({ type: Number }) row: number; - @property({ type: String }) - type: "background" | "tooltip" = "background"; + @property({ type: String, attribute: "rendered-code" }) + renderedCode: string; get code(): string { return submissionState.codeByLine[this.row - 1]; @@ -136,28 +137,28 @@ export class MarkingLayer extends ShadowlessLitElement { }); } - get codeLineClass(): string { - return this.type === "background" ? "code-line background-layer" : "code-line tooltip-layer"; - } - render(): TemplateResult { - const markedCode = this.ranges.map(range => { + const backgroundLayer = []; + const tooltipLayer = []; + + for (const range of this.ranges) { const substring = this.code.substring(range.start, range.start + range.length); if (!range.annotations.length) { - return substring; + backgroundLayer.push(substring); + tooltipLayer.push(substring); + } else { + backgroundLayer.push(html`${substring}`); + tooltipLayer.push(html`${substring}`); } - return this.type === "background" ? - html`${substring}` : - html`${substring}`; - }); + } - if (this.type === "background") { - return html` + return html` +
-
${markedCode}
-
`; - } else { - return html`
${markedCode}
`; - } +
${backgroundLayer}
+ +
${unsafeHTML(this.renderedCode)}
+
${tooltipLayer}
+
`; } } diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index c5cd005314..013d4a39cc 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -1,7 +1,6 @@ import { ShadowlessLitElement } from "components/meta/shadowless_lit_element"; import { customElement, property } from "lit/decorators.js"; import { html, TemplateResult } from "lit"; -import { unsafeHTML } from "lit/directives/unsafe-html.js"; import "components/annotations/annotations_cell"; import "components/annotations/hidden_annotations_dot"; import { i18nMixin } from "components/meta/i18n_mixin"; @@ -13,7 +12,7 @@ import { userAnnotationState } from "state/UserAnnotations"; import "components/annotations/selection_marker"; import "components/annotations/create_annotation_button"; import { triggerSelectionStart } from "components/annotations/select"; -import "components/annotations/marking_layer"; +import "components/annotations/code_layers"; /** * This component represents a row in the code listing. @@ -79,11 +78,7 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
${this.row}
-
- -
${unsafeHTML(this.renderedCode)}
- -
+ this.closeForm()} From 9ebfd902d2793b1a5b71e21bd4b7282eb3b85c7d Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jul 2023 15:13:12 +0200 Subject: [PATCH 08/27] Reintroduce selection coloring --- .../annotations/annotation_marker.ts | 22 ++++++++++++++----- .../components/annotations/code_layers.ts | 14 +++++++----- app/assets/javascripts/state/Annotations.ts | 16 ++++++++++++-- .../components/code_listing.css.scss | 5 ++++- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/components/annotations/annotation_marker.ts b/app/assets/javascripts/components/annotations/annotation_marker.ts index fa31c95e12..0a8305baf8 100644 --- a/app/assets/javascripts/components/annotations/annotation_marker.ts +++ b/app/assets/javascripts/components/annotations/annotation_marker.ts @@ -1,6 +1,14 @@ import { customElement, property } from "lit/decorators.js"; import { html, LitElement, TemplateResult } from "lit"; -import { AnnotationData, compareAnnotationOrders, isUserAnnotation } from "state/Annotations"; +import { + AnnotationData, + annotationState, + compareAnnotationOrders, + isAnnotationData, + isUserAnnotation +} from "state/Annotations"; +import { SelectedRange } from "state/UserAnnotations"; +import { MachineAnnotationData } from "state/MachineAnnotations"; /** * A marker that styles the slotted content based on the relevant annotations. * @@ -11,7 +19,7 @@ import { AnnotationData, compareAnnotationOrders, isUserAnnotation } from "state @customElement("d-annotation-marker") export class AnnotationMarker extends LitElement { @property({ type: Array }) - annotations: AnnotationData[]; + annotations: (AnnotationData | SelectedRange)[]; static colors = { "error": "var(--error-color, red)", @@ -23,8 +31,10 @@ export class AnnotationMarker extends LitElement { "question-intense": "var(--question-intense-color, orange)", }; - static getStyle(annotation: AnnotationData): string { - if (["error", "warning", "info"].includes(annotation.type)) { + static getStyle(annotation: AnnotationData | SelectedRange ): string { + if (!isAnnotationData(annotation)) { + return `background: ${AnnotationMarker.colors[annotationState.isQuestionMode ? "question-intense" : "annotation-intense"]};`; + } else if (["error", "warning", "info"].includes(annotation.type)) { return ` text-decoration: wavy underline ${AnnotationMarker.colors[annotation.type]} 1px; text-decoration-skip-ink: none; @@ -36,12 +46,12 @@ export class AnnotationMarker extends LitElement { } } - get sortedAnnotations(): AnnotationData[] { + get sortedAnnotations(): (AnnotationData | SelectedRange)[] { return this.annotations.sort( compareAnnotationOrders ); } get machineAnnotationMarkerSVG(): TemplateResult | undefined { - const firstMachineAnnotation = this.sortedAnnotations.find(a => !isUserAnnotation(a)); + const firstMachineAnnotation = this.sortedAnnotations.find(a => isAnnotationData(a) && !isUserAnnotation(a)) as MachineAnnotationData | undefined; const size = 14; return firstMachineAnnotation && html` diff --git a/app/assets/javascripts/components/annotations/code_layers.ts b/app/assets/javascripts/components/annotations/code_layers.ts index 8984495661..fe3e7cde6e 100644 --- a/app/assets/javascripts/components/annotations/code_layers.ts +++ b/app/assets/javascripts/components/annotations/code_layers.ts @@ -3,7 +3,7 @@ import { customElement, property } from "lit/decorators.js"; import { html, TemplateResult } from "lit"; import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations"; import { SelectedRange, UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; -import { AnnotationData, compareAnnotationOrders } from "state/Annotations"; +import { AnnotationData, compareAnnotationOrders, isAnnotationData } from "state/Annotations"; import { submissionState } from "state/Submissions"; import "components/annotations/annotation_marker"; import "components/annotations/annotation_tooltip"; @@ -48,8 +48,12 @@ export class CodeLayers extends ShadowlessLitElement { userAnnotationState.selectedRange.row + (userAnnotationState.selectedRange.rows ?? 1) > this.row; } - get fullLineAnnotations(): UserAnnotationData[] { - return this.userAnnotationsToMark + get fullLineAnnotations(): (UserAnnotationData | SelectedRange)[] { + const annotations: (UserAnnotationData | SelectedRange)[] = this.userAnnotationsToMark; + if (this.shouldMarkSelection) { + annotations.push(userAnnotationState.selectedRange); + } + return annotations .filter(a => !a.column && !a.columns) .sort(compareAnnotationOrders); } @@ -148,14 +152,14 @@ export class CodeLayers extends ShadowlessLitElement { tooltipLayer.push(substring); } else { backgroundLayer.push(html`${substring}`); - tooltipLayer.push(html`${substring}`); + tooltipLayer.push(html` isAnnotationData(a))}>${substring}`); } } return html`
-
${backgroundLayer}
+
${backgroundLayer}
${unsafeHTML(this.renderedCode)}
${tooltipLayer}
diff --git a/app/assets/javascripts/state/Annotations.ts b/app/assets/javascripts/state/Annotations.ts index 6a847ea795..c320fabe6f 100644 --- a/app/assets/javascripts/state/Annotations.ts +++ b/app/assets/javascripts/state/Annotations.ts @@ -1,5 +1,5 @@ import { MachineAnnotationData } from "state/MachineAnnotations"; -import { AnnotationType, UserAnnotationData } from "state/UserAnnotations"; +import { AnnotationType, SelectedRange, UserAnnotationData } from "state/UserAnnotations"; import { State } from "state/state_system/State"; import { stateProperty } from "state/state_system/StateProperty"; import { StateMap } from "state/state_system/StateMap"; @@ -15,7 +15,19 @@ const annotationOrder: Record = { info: 4, }; -export function compareAnnotationOrders(a: AnnotationData, b: AnnotationData): number { +export function isAnnotationData(annotation: AnnotationData | SelectedRange): annotation is AnnotationData { + return "type" in annotation; +} + +export function compareAnnotationOrders(a: AnnotationData | SelectedRange, b: AnnotationData | SelectedRange): number { + if (!isAnnotationData(a)) { + return -1; + } + + if (!isAnnotationData(b)) { + return 1; + } + return annotationOrder[a.type] - annotationOrder[b.type]; } diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index 4429c1dfca..d8e8d2a236 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -293,6 +293,7 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 width: 100%; height: 100%; color: transparent; + pointer-events: none; } .text-layer { @@ -301,6 +302,7 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 height: 100%; top: 0; background: transparent; + pointer-events: none; } .tooltip-layer { @@ -308,7 +310,8 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 width: 100%; height: 100%; top: 0; - opacity: 0; + color: transparent; + opacity: 0.2; } } From 62c2e5c362e3891eaae558024e2524e7785dc8e5 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 26 Jul 2023 15:15:33 +0200 Subject: [PATCH 09/27] Remove unused code --- app/assets/javascripts/mark.ts | 154 --------------------------------- app/assets/javascripts/util.js | 3 - test/javascript/mark.test.ts | 54 ------------ 3 files changed, 211 deletions(-) delete mode 100644 app/assets/javascripts/mark.ts delete mode 100644 test/javascript/mark.test.ts diff --git a/app/assets/javascripts/mark.ts b/app/assets/javascripts/mark.ts deleted file mode 100644 index a39dd823a4..0000000000 --- a/app/assets/javascripts/mark.ts +++ /dev/null @@ -1,154 +0,0 @@ -export type range = { start: number, length: number, data?: unknown }; // data is optional -type callback = (node: Node, range: range) => void; - -/** - * Returns an array of text nodes or empty wrapper nodes and their start and end indices in the root node. - * @param root The root node to search for text nodes. - * @param wrapper The type of wrapper to search for. - */ -function getTextNodes(root: Node, wrapper: string): { start: number, end: number, node: Text; }[] { - let val = ""; - const nodes = []; - const iterator = document.createNodeIterator(root, NodeFilter.SHOW_ALL, node => { - if (node.nodeType === Node.TEXT_NODE) { - return NodeFilter.FILTER_ACCEPT; - } else if (node.nodeName.toUpperCase() === wrapper.toUpperCase() && node.childNodes.length === 0) { - // Accept empty wrapper nodes - return NodeFilter.FILTER_ACCEPT; - } else { - return NodeFilter.FILTER_REJECT; - } - }); - - let node; - while (node = iterator.nextNode()) { - nodes.push({ - start: val.length, - end: (val += node.textContent).length, - node - }); - } - return nodes; -} - -/** - * Returns the closest wrapper node of the given type that is an ancestor of the given node. - * @param node The node to search for a wrapper. - * @param wrapper The type of wrapper to search for. - */ -function closestWrapper(node: Node, wrapper: string): Node | null { - let parent = node; - while (parent !== null) { - if (parent.nodeName.toUpperCase() === wrapper.toUpperCase()) { - return parent; - } - parent = parent.parentNode; - } - return null; -} - -/** - * Wraps all elements in the given range of the root node in the given wrapper node. - * For each part of the range that is already wrapped in the given wrapper node, the callback is called with the existing wrapper node. - * For each part of the range that is not wrapped in the given wrapper node, the callback is called for the newly created wrapper node. - * - * If no text nodes are found in the root node, an empty wrapper node is created and the callback is called for it. - * - * @param root The root node to search for text nodes. - * @param range The range to wrap. - * @param wrapper The type of wrapper to create. - * @param callback The callback to call for each wrapper node. - */ -function wrapRange(root: Node, range: range, wrapper: string, callback: callback): void { - const start = range.start; - const end = start + range.length; - const nodes = getTextNodes(root, wrapper); - - let wrappedLength = 0; - for (const node of nodes) { - if (node.end > start && node.start <= end && node.node.textContent !== "\n" || (range.length === 0 && node.end === start)) { - const closest = closestWrapper(node.node, wrapper); - if (closest === null) { - const splitStart = Math.max(0, start - node.start); - let nodeToWrap = node.node; - if (start > node.start) { - nodeToWrap = node.node.splitText(splitStart); - } - - if (node.end > end) { - nodeToWrap.splitText(end - node.start - splitStart); - } - const wrapperNode = document.createElement(wrapper); - wrapperNode.textContent = nodeToWrap.textContent; - nodeToWrap.parentNode.replaceChild(wrapperNode, nodeToWrap); - callback(wrapperNode, range); - - // Avoid needless wrapping of empty text nodes - wrappedLength += wrapperNode.textContent.length; - if (wrappedLength >= range.length) { - return; - } - } else { - callback(closest, range); - // Avoid needless wrapping of empty text nodes - wrappedLength += closest.textContent.length; - if (wrappedLength >= range.length) { - return; - } - } - } - } - - if (nodes.length === 0) { - const wrapperNode = document.createElement(wrapper); - wrapperNode.textContent = root.textContent; - root.appendChild(wrapperNode); - callback(wrapperNode, range); - } -} - -/** - * Wraps all elements in the given ranges of the root node in the given wrapper node. - * @param root The root node to search for text nodes. - * @param ranges The ranges to wrap. - * @param wrapper The type of wrapper to create. - * @param callback The callback to call for each wrapper node. - */ -function wrapRanges(root: Node, ranges: range[], wrapper: string, callback: callback): void { - ranges.forEach(range => wrapRange(root, range, wrapper, callback)); -} - -/** - * Todo move to utils when it is changed to typescript - * - * Returns a function that mimics the given function, but caches its results. - * This is a generic memoization function that works with any number of arguments. - * @param f The function to memoize. - */ -function cached(f: (...args: ArgumentsType) => ReturnType): (...args: ArgumentsType) => ReturnType { - const cache = new Map(); - return (...args: ArgumentsType) => { - const key = JSON.stringify(args); - if (!cache.has(key)) { - cache.set(key, f(...args)); - } - return cache.get(key); - }; -} - -/** - * Wraps all elements in the given ranges of the given target string in the given wrapper node. - * @param target a html string whose text nodes should be wrapped - * @param ranges the ranges of the textcontent to wrap - * @param wrapper the type of wrapper to create - * @param callback the callback to call for each wrapper node - */ - -export function wrapRangesInHtml(target: string, ranges: range[], wrapper: string, callback: callback = () => undefined): string { - const root = document.createElement("div"); - root.innerHTML = target; - wrapRanges(root, ranges, wrapper, callback); - return root.innerHTML; -} - -export const wrapRangesInHtmlCached = cached(wrapRangesInHtml); diff --git a/app/assets/javascripts/util.js b/app/assets/javascripts/util.js index 18a5769b5b..57d44ec512 100644 --- a/app/assets/javascripts/util.js +++ b/app/assets/javascripts/util.js @@ -226,9 +226,6 @@ function getParentByClassName(element, classNames) { return null; } -// insert `cached` function here after move to typescript -// the function is currently in `app/assets/javascripts/mark.ts` - export { createDelayer, delay, diff --git a/test/javascript/mark.test.ts b/test/javascript/mark.test.ts deleted file mode 100644 index fa6caedd48..0000000000 --- a/test/javascript/mark.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { wrapRangesInHtml } from "mark"; - -test("marking a range in an empty string should still create a marker", () => { - const result = wrapRangesInHtml("", [{ start: 0, length: 1 }], "mark", () => undefined); - expect(result).toBe(""); -}); - -test("marking a range in a string should create a marker", () => { - const result = wrapRangesInHtml("Hello World", [{ start: 0, length: 5 }], "mark", () => undefined); - expect(result).toBe("Hello World"); -}); - -test("marking a range in a complex html string should create a marker", () => { - const result = wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 5 }], "mark", () => undefined); - expect(result).toBe("
Hel
lo Wo
rld
"); -}); - -test("Callback should be called for each marker", () => { - const callback = jest.fn(); - wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 5 }], "mark", callback); - expect(callback).toHaveBeenCalledTimes(4); -}); - -test("should be able to mark multiple ranges", () => { - const result = wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 5 }, { start: 6, length: 2 }], "mark", () => undefined); - expect(result).toBe("
Hel
lo Wo
rld
"); -}); - -test("should be able to mark multiple ranges with overlapping ranges", () => { - const result = wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 5 }, { start: 2, length: 2 }], "mark", () => undefined); - expect(result).toBe("
Hel
lo Wo
rld
"); -}); - -test("Callback should be called for each marker for each range", () => { - const callback = jest.fn(); - wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 5 }, { start: 2, length: 2 }], "mark", callback); - expect(callback).toHaveBeenCalledTimes(6); -}); - -test("should be able to edit attributes of the marker in the callback", () => { - const callback = jest.fn((element, range) => { - const test = element.getAttribute("data-test") || ""; - element.setAttribute("data-test", test + range.data); - }); - const result = wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 5, data: "foo" }, { start: 2, length: 7, data: "bar" }], "mark", callback); - expect(callback).toHaveBeenCalledWith(expect.any(HTMLElement), { start: 0, length: 5, data: "foo" }); - expect(callback).toHaveBeenCalledWith(expect.any(HTMLElement), { start: 2, length: 7, data: "bar" }); - expect(result).toBe("
Hel
lo Wo
rld
"); -}); - -test("should be able to mark zero length ranges", () => { - const result = wrapRangesInHtml("
Hel
lo Wo
rld
", [{ start: 0, length: 0 }, { start: 5, length: 0 }, { start: 8, length: 0 }, { start: 11, length: 0 }], "mark", () => undefined); - expect(result).toBe("
Hel
lo Wo
rld
"); -}); From 41fbc4cfbc4ad8fd094eb45dc5bb86433d109c8b Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 09:42:27 +0200 Subject: [PATCH 10/27] Fix selection in chrome --- .../components/annotations/select.ts | 8 ++-- .../components/code_listing.css.scss | 37 ++----------------- 2 files changed, 7 insertions(+), 38 deletions(-) diff --git a/app/assets/javascripts/components/annotations/select.ts b/app/assets/javascripts/components/annotations/select.ts index 52cd94b6f4..b7004c29b6 100644 --- a/app/assets/javascripts/components/annotations/select.ts +++ b/app/assets/javascripts/components/annotations/select.ts @@ -133,15 +133,15 @@ export function selectedRangeFromSelection(selection: Selection): SelectedRange const newRange = new Range(); const startLine = document.querySelector(`#line-${range.row}`); const endLine = document.querySelector(`#line-${range.row + range.rows - 1}`); - newRange.setStart(startLine.querySelector(".code-line"), 0); - newRange.setEnd(endLine.querySelector(".code-line"), endLine.querySelector(".code-line").childNodes.length); + newRange.setStart(startLine.querySelector(".tooltip-layer"), 0); + newRange.setEnd(endLine.querySelector(".tooltip-layer"), endLine.querySelector(".tooltip-layer").childNodes.length); selection.addRange(newRange); } else { for (let i = range.row; i < range.row + range.rows; i++) { const newRange = new Range(); const line = document.querySelector(`#line-${i}`); - newRange.setStart(line.querySelector(".code-line"), 0); - newRange.setEnd(line.querySelector(".code-line"), line.querySelector(".code-line").childNodes.length); + newRange.setStart(line.querySelector(".tooltip-layer"), 0); + newRange.setEnd(line.querySelector(".tooltip-layer"), line.querySelector(".tooltip-layer").childNodes.length); selection.addRange(newRange); } } diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index d8e8d2a236..dead10f418 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -113,39 +113,6 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 --question-color: #{$warning-container}; --annotation-color: #{$warning-container}; - - pre { - background-color: inherit; - - &.code-line-question { - background-color: $annotation-question-background-intense; - } - - &.code-line-annotation { - background-color: $annotation-user-background-intense; - } - } - } - - // Highlight fully selected lines, also force all annotations in the line to the selection color - .lineno .rouge-code pre { - &.code-line-question { - background-color: $annotation-question-background-intense; - width: 100%; - - d-annotation-marker { - background-color: $annotation-question-background-intense; - } - } - - &.code-line-annotation { - background-color: $annotation-user-background-intense; - width: 100%; - - d-annotation-marker { - background-color: $annotation-user-background-intense; - } - } } .annotation-button { @@ -294,6 +261,7 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 height: 100%; color: transparent; pointer-events: none; + user-select: none; } .text-layer { @@ -303,6 +271,7 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 top: 0; background: transparent; pointer-events: none; + user-select: none; } .tooltip-layer { @@ -311,7 +280,7 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 height: 100%; top: 0; color: transparent; - opacity: 0.2; + opacity: 0.4; } } From 0f7e8b6a3de673e88aa4b50b07e6f15593eb9c2a Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 11:22:58 +0200 Subject: [PATCH 11/27] speedup code using selection layer --- .../annotations/annotation_marker.ts | 14 ++--- .../components/annotations/code_layers.ts | 35 +++++------- .../annotations/code_listing_row.ts | 1 - .../components/annotations/selection_layer.ts | 53 +++++++++++++++++++ .../annotations/selection_marker.ts | 25 --------- app/assets/javascripts/state/Annotations.ts | 17 +----- .../components/code_listing.css.scss | 18 +++++++ 7 files changed, 90 insertions(+), 73 deletions(-) create mode 100644 app/assets/javascripts/components/annotations/selection_layer.ts delete mode 100644 app/assets/javascripts/components/annotations/selection_marker.ts diff --git a/app/assets/javascripts/components/annotations/annotation_marker.ts b/app/assets/javascripts/components/annotations/annotation_marker.ts index 0a8305baf8..16329cc66f 100644 --- a/app/assets/javascripts/components/annotations/annotation_marker.ts +++ b/app/assets/javascripts/components/annotations/annotation_marker.ts @@ -4,10 +4,8 @@ import { AnnotationData, annotationState, compareAnnotationOrders, - isAnnotationData, isUserAnnotation } from "state/Annotations"; -import { SelectedRange } from "state/UserAnnotations"; import { MachineAnnotationData } from "state/MachineAnnotations"; /** * A marker that styles the slotted content based on the relevant annotations. @@ -19,7 +17,7 @@ import { MachineAnnotationData } from "state/MachineAnnotations"; @customElement("d-annotation-marker") export class AnnotationMarker extends LitElement { @property({ type: Array }) - annotations: (AnnotationData | SelectedRange)[]; + annotations: AnnotationData[]; static colors = { "error": "var(--error-color, red)", @@ -31,10 +29,8 @@ export class AnnotationMarker extends LitElement { "question-intense": "var(--question-intense-color, orange)", }; - static getStyle(annotation: AnnotationData | SelectedRange ): string { - if (!isAnnotationData(annotation)) { - return `background: ${AnnotationMarker.colors[annotationState.isQuestionMode ? "question-intense" : "annotation-intense"]};`; - } else if (["error", "warning", "info"].includes(annotation.type)) { + static getStyle(annotation: AnnotationData): string { + if (["error", "warning", "info"].includes(annotation.type)) { return ` text-decoration: wavy underline ${AnnotationMarker.colors[annotation.type]} 1px; text-decoration-skip-ink: none; @@ -46,12 +42,12 @@ export class AnnotationMarker extends LitElement { } } - get sortedAnnotations(): (AnnotationData | SelectedRange)[] { + get sortedAnnotations(): AnnotationData[] { return this.annotations.sort( compareAnnotationOrders ); } get machineAnnotationMarkerSVG(): TemplateResult | undefined { - const firstMachineAnnotation = this.sortedAnnotations.find(a => isAnnotationData(a) && !isUserAnnotation(a)) as MachineAnnotationData | undefined; + const firstMachineAnnotation = this.sortedAnnotations.find(a => !isUserAnnotation(a)) as MachineAnnotationData | undefined; const size = 14; return firstMachineAnnotation && html` diff --git a/app/assets/javascripts/components/annotations/code_layers.ts b/app/assets/javascripts/components/annotations/code_layers.ts index fe3e7cde6e..71d090ca29 100644 --- a/app/assets/javascripts/components/annotations/code_layers.ts +++ b/app/assets/javascripts/components/annotations/code_layers.ts @@ -2,17 +2,18 @@ import { ShadowlessLitElement } from "components/meta/shadowless_lit_element"; import { customElement, property } from "lit/decorators.js"; import { html, TemplateResult } from "lit"; import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations"; -import { SelectedRange, UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; -import { AnnotationData, compareAnnotationOrders, isAnnotationData } from "state/Annotations"; +import { UserAnnotationData, userAnnotationState } from "state/UserAnnotations"; +import { AnnotationData, compareAnnotationOrders } from "state/Annotations"; import { submissionState } from "state/Submissions"; import "components/annotations/annotation_marker"; import "components/annotations/annotation_tooltip"; +import "components/annotations/selection_layer"; import { unsafeHTML } from "lit/directives/unsafe-html.js"; declare type range = { start: number; length: number; - annotations: (AnnotationData | SelectedRange)[]; + annotations: AnnotationData[]; }; function numberArrayEquals(a: number[], b: number[]): boolean { @@ -42,18 +43,8 @@ export class CodeLayers extends ShadowlessLitElement { return userAnnotationState.rootIdsByMarkedLine.get(this.row)?.map(i => userAnnotationState.byId.get(i)) || []; } - get shouldMarkSelection(): boolean { - return userAnnotationState.selectedRange && - userAnnotationState.selectedRange.row <= this.row && - userAnnotationState.selectedRange.row + (userAnnotationState.selectedRange.rows ?? 1) > this.row; - } - - get fullLineAnnotations(): (UserAnnotationData | SelectedRange)[] { - const annotations: (UserAnnotationData | SelectedRange)[] = this.userAnnotationsToMark; - if (this.shouldMarkSelection) { - annotations.push(userAnnotationState.selectedRange); - } - return annotations + get fullLineAnnotations(): UserAnnotationData[] { + return this.userAnnotationsToMark .filter(a => !a.column && !a.columns) .sort(compareAnnotationOrders); } @@ -64,8 +55,8 @@ export class CodeLayers extends ShadowlessLitElement { * In that case, the range will be the part of the line that is covered by the annotation. * @param annotation The annotation to calculate the range for. */ - getRangeFromAnnotation(annotation: AnnotationData | SelectedRange, index: number): { start: number, length: number, index: number } { - const isMachineAnnotation = ["error", "warning", "info"].includes((annotation as AnnotationData).type); + getRangeFromAnnotation(annotation: AnnotationData, index: number): { start: number, length: number, index: number } { + const isMachineAnnotation = ["error", "warning", "info"].includes(annotation.type); const rowsLength = annotation.rows ?? 1; let lastRow = annotation.row ? annotation.row + rowsLength : 0; let firstRow = annotation.row ? annotation.row + 1 : 0; @@ -128,10 +119,7 @@ export class CodeLayers extends ShadowlessLitElement { } get ranges(): range[] { - const toMark: (AnnotationData | SelectedRange)[] = [...this.machineAnnotationsToMark, ...this.userAnnotationsToMark]; - if (this.shouldMarkSelection) { - toMark.push(userAnnotationState.selectedRange); - } + const toMark: AnnotationData[] = [...this.machineAnnotationsToMark, ...this.userAnnotationsToMark]; // We use indexes to simplify the equality check in mergeRanges const ranges = toMark.map((annotation, index) => this.getRangeFromAnnotation(annotation, index)); const mergedRanges = this.mergeRanges(ranges); @@ -152,15 +140,16 @@ export class CodeLayers extends ShadowlessLitElement { tooltipLayer.push(substring); } else { backgroundLayer.push(html`${substring}`); - tooltipLayer.push(html` isAnnotationData(a))}>${substring}`); + tooltipLayer.push(html`${substring}`); } } return html`
- +
${backgroundLayer}
+
${unsafeHTML(this.renderedCode)}
${tooltipLayer}
`; diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index 013d4a39cc..f65ff4f2c2 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -9,7 +9,6 @@ import { PropertyValues } from "@lit/reactive-element"; import { userState } from "state/Users"; import { annotationState } from "state/Annotations"; import { userAnnotationState } from "state/UserAnnotations"; -import "components/annotations/selection_marker"; import "components/annotations/create_annotation_button"; import { triggerSelectionStart } from "components/annotations/select"; import "components/annotations/code_layers"; diff --git a/app/assets/javascripts/components/annotations/selection_layer.ts b/app/assets/javascripts/components/annotations/selection_layer.ts new file mode 100644 index 0000000000..cb569174eb --- /dev/null +++ b/app/assets/javascripts/components/annotations/selection_layer.ts @@ -0,0 +1,53 @@ +import { html, TemplateResult } from "lit"; +import { customElement, property } from "lit/decorators.js"; +import { userAnnotationState } from "state/UserAnnotations"; +import { ShadowlessLitElement } from "components/meta/shadowless_lit_element"; +import { submissionState } from "state/Submissions"; +import { annotationState } from "state/Annotations"; + +/** + * A separate layer that contains the marking for the selection. + * It is separate from code-layers to make the rendering independent and thus much faster. + * + * The selected code is styled to look like a user annotation. + * It is used to mark the selected code while editing an annotation + * @element d-selection-layer + */ +@customElement("d-selection-layer") +class SelectionLayer extends ShadowlessLitElement { + @property({ type: Number }) + row: number; + + get code(): string { + return submissionState.codeByLine[this.row - 1]; + } + + + get shouldMarkSelection(): boolean { + return userAnnotationState.selectedRange && + userAnnotationState.selectedRange.row <= this.row && + userAnnotationState.selectedRange.row + (userAnnotationState.selectedRange.rows ?? 1) > this.row; + } + + get markingClass(): string { + return annotationState.isQuestionMode ? "question-selection-marker" : "annotation-selection-marker"; + } + + render(): TemplateResult { + if (!this.shouldMarkSelection) { + return html``; + } + if (!userAnnotationState.selectedRange.column && !userAnnotationState.selectedRange.columns) { + return html`
${this.code}
`; + } + + const start = userAnnotationState.selectedRange.column ?? 0; + const end = userAnnotationState.selectedRange.columns ? start + userAnnotationState.selectedRange.columns : this.code.length; + + return html`
${
+            this.code.substring(0, start)}${
+            html`${this.code.substring(start, end)}`}${
+            this.code.substring(end)
+        }
`; + } +} diff --git a/app/assets/javascripts/components/annotations/selection_marker.ts b/app/assets/javascripts/components/annotations/selection_marker.ts deleted file mode 100644 index 71d1e24072..0000000000 --- a/app/assets/javascripts/components/annotations/selection_marker.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { LitElement, html, TemplateResult } from "lit"; -import { customElement } from "lit/decorators.js"; -import { AnnotationMarker } from "components/annotations/annotation_marker"; -import { annotationState } from "state/Annotations"; - -/** - * A marker that replaces the selection. - * The slotted content is styled to look like a user annotation. - * It is used to mark the selected code while editing an annotation - * @element d-selection-marker - */ -@customElement("d-selection-marker") -class SelectionMarker extends LitElement { - render(): TemplateResult { - return html``; - } -} diff --git a/app/assets/javascripts/state/Annotations.ts b/app/assets/javascripts/state/Annotations.ts index c320fabe6f..f7626c355d 100644 --- a/app/assets/javascripts/state/Annotations.ts +++ b/app/assets/javascripts/state/Annotations.ts @@ -1,8 +1,7 @@ import { MachineAnnotationData } from "state/MachineAnnotations"; -import { AnnotationType, SelectedRange, UserAnnotationData } from "state/UserAnnotations"; +import { AnnotationType, UserAnnotationData } from "state/UserAnnotations"; import { State } from "state/state_system/State"; import { stateProperty } from "state/state_system/StateProperty"; -import { StateMap } from "state/state_system/StateMap"; export type AnnotationVisibilityOptions = "all" | "important" | "none"; export type AnnotationData = MachineAnnotationData | UserAnnotationData; @@ -15,19 +14,7 @@ const annotationOrder: Record = { info: 4, }; -export function isAnnotationData(annotation: AnnotationData | SelectedRange): annotation is AnnotationData { - return "type" in annotation; -} - -export function compareAnnotationOrders(a: AnnotationData | SelectedRange, b: AnnotationData | SelectedRange): number { - if (!isAnnotationData(a)) { - return -1; - } - - if (!isAnnotationData(b)) { - return 1; - } - +export function compareAnnotationOrders(a: AnnotationData, b: AnnotationData): number { return annotationOrder[a.type] - annotationOrder[b.type]; } diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index dead10f418..90f44fdcc1 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -264,6 +264,16 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 user-select: none; } + .selection-layer { + position: absolute; + width: 100%; + height: 100%; + top: 0; + color: transparent; + pointer-events: none; + user-select: none; + } + .text-layer { position: absolute; width: 100%; @@ -290,6 +300,14 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 padding-top: 1px; } + .question-selection-marker { + background-color: $annotation-question-background-intense; + } + + .annotation-selection-marker { + background-color: $annotation-user-background-intense; + } + // This is a hack to extend the drop target more to the left .drop-target-extension { position: absolute; From f08d3d8f491e8b04c3d26c5a15416c4a7522d8eb Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 11:37:46 +0200 Subject: [PATCH 12/27] Improve live selection coloring --- app/assets/javascripts/components/annotations/code_layers.ts | 2 +- app/assets/stylesheets/components/code_listing.css.scss | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/components/annotations/code_layers.ts b/app/assets/javascripts/components/annotations/code_layers.ts index 71d090ca29..806e61382c 100644 --- a/app/assets/javascripts/components/annotations/code_layers.ts +++ b/app/assets/javascripts/components/annotations/code_layers.ts @@ -150,8 +150,8 @@ export class CodeLayers extends ShadowlessLitElement {
${backgroundLayer}
-
${unsafeHTML(this.renderedCode)}
${tooltipLayer}
+
${unsafeHTML(this.renderedCode)}
`; } } diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index 90f44fdcc1..e2b2f45bfb 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -290,7 +290,6 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 height: 100%; top: 0; color: transparent; - opacity: 0.4; } } From eadc77da83ff9ea6b8da479ef709893834252b07 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 11:56:56 +0200 Subject: [PATCH 13/27] Fix tests --- .../components/annotations/select.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/javascript/components/annotations/select.test.ts b/test/javascript/components/annotations/select.test.ts index 9e09c571ce..72a4cfa49a 100644 --- a/test/javascript/components/annotations/select.test.ts +++ b/test/javascript/components/annotations/select.test.ts @@ -32,6 +32,7 @@ describe("getOffsetTest", () => { describe("selectedRangeFromSelectionTest", () => { let context; beforeEach(async () => { + submissionState.code = "hello world\n\nprint(world)"; context = await fixture(`
bar @@ -40,7 +41,6 @@ describe("selectedRangeFromSelectionTest", () => {
`); - submissionState.code = "hello world\n\nprint(world)"; window.getSelection().removeAllRanges(); }); @@ -69,10 +69,10 @@ describe("selectedRangeFromSelectionTest", () => { expect(selection.rangeCount).toBe(1); const newRange = selection.getRangeAt(0); - expect(newRange.startContainer).toBe(context.querySelector("#line-1 .code-line")); - expect(newRange.endContainer).toBe(context.querySelector("#line-3 .code-line")); + expect(newRange.startContainer).toBe(context.querySelector("#line-1 .tooltip-layer")); + expect(newRange.endContainer).toBe(context.querySelector("#line-3 .tooltip-layer")); expect(newRange.startOffset).toBe(0); - expect(newRange.endOffset).toBe(5); + expect(newRange.endOffset).toBe(4); }); it("Should create multiple ranges if the selection contains multiple ranges", async () => { @@ -126,10 +126,10 @@ describe("selectedRangeFromSelectionTest", () => { expect(selection.rangeCount).toBe(1); const newRange = selection.getRangeAt(0); - expect(newRange.startContainer).toBe(context.querySelector("#line-3 .code-line")); - expect(newRange.endContainer).toBe(context.querySelector("#line-3 .code-line")); + expect(newRange.startContainer).toBe(context.querySelector("#line-3 .tooltip-layer")); + expect(newRange.endContainer).toBe(context.querySelector("#line-3 .tooltip-layer")); expect(newRange.startOffset).toBe(0); - expect(newRange.endOffset).toBe(5); + expect(newRange.endOffset).toBe(4); }); it("should remove ending empty lines from the selection", async () => { From 9c6d99956da38a1523239f4458d18a9ab70879e8 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 13:25:49 +0200 Subject: [PATCH 14/27] Add comments --- .../javascripts/components/annotations/code_layers.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/assets/javascripts/components/annotations/code_layers.ts b/app/assets/javascripts/components/annotations/code_layers.ts index 806e61382c..ac1b605d48 100644 --- a/app/assets/javascripts/components/annotations/code_layers.ts +++ b/app/assets/javascripts/components/annotations/code_layers.ts @@ -20,6 +20,14 @@ function numberArrayEquals(a: number[], b: number[]): boolean { return a.length === b.length && a.every((v, i) => v === b[i]); } +/** + * renders the code with formatting and tooltips split in separate layers + * + * @prop {number} row - the row of the code to render + * @prop {string} renderedCode - the syntax highlighted code to render + * + * @element d-code-layers + */ @customElement("d-code-layers") export class CodeLayers extends ShadowlessLitElement { @property({ type: Number }) From 3c88930f2fb706a957fb6678357c5c568ee19e17 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 15:33:49 +0200 Subject: [PATCH 15/27] Rewrite selection layer --- .../components/annotations/selection_layer.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/components/annotations/selection_layer.ts b/app/assets/javascripts/components/annotations/selection_layer.ts index cb569174eb..74e1dbf9a9 100644 --- a/app/assets/javascripts/components/annotations/selection_layer.ts +++ b/app/assets/javascripts/components/annotations/selection_layer.ts @@ -44,10 +44,13 @@ class SelectionLayer extends ShadowlessLitElement { const start = userAnnotationState.selectedRange.column ?? 0; const end = userAnnotationState.selectedRange.columns ? start + userAnnotationState.selectedRange.columns : this.code.length; - return html`
${
-            this.code.substring(0, start)}${
-            html`${this.code.substring(start, end)}`}${
+        const selectionLayer: (TemplateResult | string)[] = [
+            html``, // this is a hack to force the height of the line to be correct even if no code is on this line
+            this.code.substring(0, start),
+            html`${this.code.substring(start, end)}`,
             this.code.substring(end)
-        }
`; + ]; + + return html`
${selectionLayer}
`; } } From 4e30fb3f03afa0ba0e9ad7f9212ce2e3f3022176 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 15:36:13 +0200 Subject: [PATCH 16/27] Rename code-listing --- .../annotations/{code_layers.ts => code_listing.ts} | 6 +++--- .../javascripts/components/annotations/code_listing_row.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) rename app/assets/javascripts/components/annotations/{code_layers.ts => code_listing.ts} (98%) diff --git a/app/assets/javascripts/components/annotations/code_layers.ts b/app/assets/javascripts/components/annotations/code_listing.ts similarity index 98% rename from app/assets/javascripts/components/annotations/code_layers.ts rename to app/assets/javascripts/components/annotations/code_listing.ts index ac1b605d48..9133a63431 100644 --- a/app/assets/javascripts/components/annotations/code_layers.ts +++ b/app/assets/javascripts/components/annotations/code_listing.ts @@ -26,10 +26,10 @@ function numberArrayEquals(a: number[], b: number[]): boolean { * @prop {number} row - the row of the code to render * @prop {string} renderedCode - the syntax highlighted code to render * - * @element d-code-layers + * @element d-code-listing */ -@customElement("d-code-layers") -export class CodeLayers extends ShadowlessLitElement { +@customElement("d-code-listing") +export class CodeListing extends ShadowlessLitElement { @property({ type: Number }) row: number; @property({ type: String, attribute: "rendered-code" }) diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index f65ff4f2c2..36578a4741 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -11,7 +11,7 @@ import { annotationState } from "state/Annotations"; import { userAnnotationState } from "state/UserAnnotations"; import "components/annotations/create_annotation_button"; import { triggerSelectionStart } from "components/annotations/select"; -import "components/annotations/code_layers"; +import "components/annotations/code_listing"; /** * This component represents a row in the code listing. @@ -77,7 +77,7 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
${this.row}
- + this.closeForm()} From 384d8319e1a3c0482b0e142e465f53d69c38aa87 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 15:37:59 +0200 Subject: [PATCH 17/27] remove unused code --- .../javascripts/components/annotations/annotation_tooltip.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/components/annotations/annotation_tooltip.ts b/app/assets/javascripts/components/annotations/annotation_tooltip.ts index 3211e27038..6d2c2b5a37 100644 --- a/app/assets/javascripts/components/annotations/annotation_tooltip.ts +++ b/app/assets/javascripts/components/annotations/annotation_tooltip.ts @@ -87,7 +87,8 @@ export class AnnotationTooltip extends LitElement { render(): TemplateResult { this.renderTooltip(); - const size = 14; + + // if slot is empty, render an empty svg to make sure the tooltip is positioned correctly return html` `; } From 637700f4d685fa6879e2b4eb47f3aeeae8261ebe Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Thu, 27 Jul 2023 16:09:28 +0200 Subject: [PATCH 18/27] group css --- .../components/code_listing.css.scss | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index e2b2f45bfb..ce64c20d85 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -256,41 +256,37 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 width: 100%; height: 100%; - .background-layer { - width: 100%; - height: 100%; - color: transparent; - pointer-events: none; - user-select: none; - } - - .selection-layer { - position: absolute; + .background-layer, + .selection-layer, + .text-layer, + .tooltip-layer { width: 100%; height: 100%; top: 0; - color: transparent; - pointer-events: none; - user-select: none; } + .background-layer, + .selection-layer, .text-layer { - position: absolute; - width: 100%; - height: 100%; - top: 0; - background: transparent; pointer-events: none; user-select: none; } + .background-layer, + .selection-layer, .tooltip-layer { - position: absolute; - width: 100%; - height: 100%; - top: 0; color: transparent; } + + .selection-layer, + .text-layer, + .tooltip-layer { + position: absolute; + } + + .text-layer { + background: transparent; + } } pre.code-line { From e4c75b275d3d1605561bba364604982447ebe3f6 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 09:57:42 +0200 Subject: [PATCH 19/27] Simplify css --- .../components/code_listing.css.scss | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index ce64c20d85..b0129b61f8 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -260,33 +260,28 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 .selection-layer, .text-layer, .tooltip-layer { + position: absolute; width: 100%; height: 100%; top: 0; - } - - .background-layer, - .selection-layer, - .text-layer { pointer-events: none; user-select: none; - } - - .background-layer, - .selection-layer, - .tooltip-layer { color: transparent; } - .selection-layer, - .text-layer, .tooltip-layer { - position: absolute; + pointer-events: auto; + user-select: auto; } .text-layer { + color: inherit; background: transparent; } + + .background-layer { + position: initial; + } } pre.code-line { From dbc4dfa4129bafae699f2f743d53f8756bd77b27 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 15:35:05 +0200 Subject: [PATCH 20/27] Rename line of code --- .../javascripts/components/annotations/code_listing_row.ts | 4 ++-- .../annotations/{code_listing.ts => line_of_code.ts} | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename app/assets/javascripts/components/annotations/{code_listing.ts => line_of_code.ts} (98%) diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index 36578a4741..824760b9de 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -11,7 +11,7 @@ import { annotationState } from "state/Annotations"; import { userAnnotationState } from "state/UserAnnotations"; import "components/annotations/create_annotation_button"; import { triggerSelectionStart } from "components/annotations/select"; -import "components/annotations/code_listing"; +import "components/annotations/line_of_code"; /** * This component represents a row in the code listing. @@ -77,7 +77,7 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
${this.row}
- + this.closeForm()} diff --git a/app/assets/javascripts/components/annotations/code_listing.ts b/app/assets/javascripts/components/annotations/line_of_code.ts similarity index 98% rename from app/assets/javascripts/components/annotations/code_listing.ts rename to app/assets/javascripts/components/annotations/line_of_code.ts index 9133a63431..c924fd2c64 100644 --- a/app/assets/javascripts/components/annotations/code_listing.ts +++ b/app/assets/javascripts/components/annotations/line_of_code.ts @@ -28,8 +28,8 @@ function numberArrayEquals(a: number[], b: number[]): boolean { * * @element d-code-listing */ -@customElement("d-code-listing") -export class CodeListing extends ShadowlessLitElement { +@customElement("d-line-of-code") +export class LineOfCode extends ShadowlessLitElement { @property({ type: Number }) row: number; @property({ type: String, attribute: "rendered-code" }) From 49a34047544c276f0f8e68cd1d2610cb05641a2d Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 15:35:53 +0200 Subject: [PATCH 21/27] Rename selection helpers --- app/assets/javascripts/code_listing.ts | 2 +- .../javascripts/components/annotations/code_listing_row.ts | 2 +- .../components/annotations/{select.ts => selectionHelpers.ts} | 0 test/javascript/components/annotations/select.test.ts | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename app/assets/javascripts/components/annotations/{select.ts => selectionHelpers.ts} (100%) diff --git a/app/assets/javascripts/code_listing.ts b/app/assets/javascripts/code_listing.ts index 0e0e71c412..aaaca43403 100644 --- a/app/assets/javascripts/code_listing.ts +++ b/app/assets/javascripts/code_listing.ts @@ -9,7 +9,7 @@ import "components/annotations/annotation_options"; import "components/annotations/annotations_count_badge"; import { annotationState } from "state/Annotations"; import { exerciseState } from "state/Exercises"; -import { triggerSelectionEnd } from "components/annotations/select"; +import { triggerSelectionEnd } from "components/annotations/selectionHelpers"; const MARKING_CLASS = "marked"; diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index 824760b9de..3563b940c1 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -10,7 +10,7 @@ import { userState } from "state/Users"; import { annotationState } from "state/Annotations"; import { userAnnotationState } from "state/UserAnnotations"; import "components/annotations/create_annotation_button"; -import { triggerSelectionStart } from "components/annotations/select"; +import { triggerSelectionStart } from "components/annotations/selectionHelpers"; import "components/annotations/line_of_code"; /** diff --git a/app/assets/javascripts/components/annotations/select.ts b/app/assets/javascripts/components/annotations/selectionHelpers.ts similarity index 100% rename from app/assets/javascripts/components/annotations/select.ts rename to app/assets/javascripts/components/annotations/selectionHelpers.ts diff --git a/test/javascript/components/annotations/select.test.ts b/test/javascript/components/annotations/select.test.ts index 72a4cfa49a..9a00199b2b 100644 --- a/test/javascript/components/annotations/select.test.ts +++ b/test/javascript/components/annotations/select.test.ts @@ -1,5 +1,5 @@ import { fixture } from "@open-wc/testing-helpers"; -import { getOffset, selectedRangeFromSelection } from "components/annotations/select"; +import { getOffset, selectedRangeFromSelection } from "components/annotations/selectionHelpers"; import "components/annotations/code_listing_row"; import { submissionState } from "state/Submissions"; From 2ef718d97c323f2cafc4646a78875ad36a395beb Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 15:40:24 +0200 Subject: [PATCH 22/27] Add comments to codeline and clarify code --- .../components/annotations/line_of_code.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/components/annotations/line_of_code.ts b/app/assets/javascripts/components/annotations/line_of_code.ts index c924fd2c64..c432ab6e0b 100644 --- a/app/assets/javascripts/components/annotations/line_of_code.ts +++ b/app/assets/javascripts/components/annotations/line_of_code.ts @@ -21,7 +21,11 @@ function numberArrayEquals(a: number[], b: number[]): boolean { } /** - * renders the code with formatting and tooltips split in separate layers + * renders the code with formatting and tooltips split into four layers: + * - the background layer renders the markup for annotations on the code (background color and underline) + * - the selection layer renders the background color for the selection (split off because keeping selection logic separate from existing annotations makes it much faster) + * - the tooltip layer handles all pointer events (text selection and tooltips) + * - the text layer renders the code with syntax highlighting * * @prop {number} row - the row of the code to render * @prop {string} renderedCode - the syntax highlighted code to render @@ -154,9 +158,12 @@ export class LineOfCode extends ShadowlessLitElement { return html`
- -
${backgroundLayer}
-
+ ${ this.fullLineAnnotations.length > 0 ? html` + +
${backgroundLayer}
+
` : html` +
${backgroundLayer}
+ `}
${tooltipLayer}
${unsafeHTML(this.renderedCode)}
From 71187b90c7d0fd6d607909a2fc156f141a1598c0 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 15:47:18 +0200 Subject: [PATCH 23/27] Improve understanadibility of annotation_tooltip --- .../annotations/annotation_tooltip.ts | 57 ++++++++++++------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/components/annotations/annotation_tooltip.ts b/app/assets/javascripts/components/annotations/annotation_tooltip.ts index 6d2c2b5a37..e97d3f0a5c 100644 --- a/app/assets/javascripts/components/annotations/annotation_tooltip.ts +++ b/app/assets/javascripts/components/annotations/annotation_tooltip.ts @@ -22,6 +22,7 @@ export class AnnotationTooltip extends LitElement { state = new StateController(this); + // we need to keep track of all tippy instances to update the singleton static tippyInstances: Tippy[] = []; // Using a singleton to avoid multiple tooltips being open at the same time. static tippySingleton = createSingleton([], { @@ -34,59 +35,71 @@ export class AnnotationTooltip extends LitElement { moveTransition: "transform 0.001s ease-out", appendTo: () => document.querySelector(".code-table"), }); + + /** + * Updates the tippy singleton with the current list of tippy instances. + * This method is debounced to avoid updating the singleton too often as it is expensive. + */ static updateSingletonInstances(): void { setInstancesDelayer(() => this.tippySingleton.setInstances(this.tippyInstances), 100); } + + /** + * Adds a tippy instance to the list of instances, which will be used to update the singleton. + */ static registerTippyInstance(instance: Tippy): void { this.tippyInstances.push(instance); this.updateSingletonInstances(); } + + /** + * Removes a tippy instance from the list of instances, which will be used to update the singleton. + */ static unregisterTippyInstance(instance: Tippy): void { this.tippyInstances = this.tippyInstances.filter(i => i !== instance); this.updateSingletonInstances(); } - // Annotations that are displayed inline should show up as tooltips. + // Annotations that are not displayed inline should show up as tooltips. get hiddenAnnotations(): AnnotationData[] { return this.annotations.filter(a => !annotationState.isVisible(a)).sort(compareAnnotationOrders); } tippyInstance: Tippy; - renderTooltip(): void { + disconnectedCallback(): void { + super.disconnectedCallback(); + // before destroying this element, we need to clean up the tippy instance + // and make sure it is removed from the singleton if (this.tippyInstance) { AnnotationTooltip.unregisterTippyInstance(this.tippyInstance); this.tippyInstance.destroy(); this.tippyInstance = undefined; } - - if (this.hiddenAnnotations.length === 0) { - return; - } - - const tooltip = document.createElement("div"); - tooltip.classList.add("marker-tooltip"); - render(this.hiddenAnnotations.map(a => isUserAnnotation(a) ? - html`` : - html``), tooltip); - - this.tippyInstance = tippy(this, { - content: tooltip, - }); - AnnotationTooltip.registerTippyInstance(this.tippyInstance); } - disconnectedCallback(): void { - super.disconnectedCallback(); + render(): TemplateResult { + // Clean up the previous tippy instance if it exists. if (this.tippyInstance) { AnnotationTooltip.unregisterTippyInstance(this.tippyInstance); this.tippyInstance.destroy(); this.tippyInstance = undefined; } - } - render(): TemplateResult { - this.renderTooltip(); + if (this.hiddenAnnotations.length > 0) { + const tooltip = document.createElement("div"); + tooltip.classList.add("marker-tooltip"); + render(this.hiddenAnnotations.map(a => isUserAnnotation(a) ? + html` + ` : + html` + `), tooltip); + + this.tippyInstance = tippy(this, { + content: tooltip, + }); + AnnotationTooltip.registerTippyInstance(this.tippyInstance); + } // if slot is empty, render an empty svg to make sure the tooltip is positioned correctly return html` From b9df2d575d7d6db816c22a95561ffda89e94e669 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 15:50:09 +0200 Subject: [PATCH 24/27] Add comments to annotation marker --- .../javascripts/components/annotations/annotation_marker.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/components/annotations/annotation_marker.ts b/app/assets/javascripts/components/annotations/annotation_marker.ts index 16329cc66f..3d88d6d541 100644 --- a/app/assets/javascripts/components/annotations/annotation_marker.ts +++ b/app/assets/javascripts/components/annotations/annotation_marker.ts @@ -9,6 +9,7 @@ import { import { MachineAnnotationData } from "state/MachineAnnotations"; /** * A marker that styles the slotted content based on the relevant annotations. + * It applies a background color to user annotations and a wavy underline to machine annotations. * * @prop {AnnotationData[]} annotations The annotations to use for styling. * From 3c5741f610cf199af305484a26f6de2922b0bd94 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Fri, 28 Jul 2023 15:51:19 +0200 Subject: [PATCH 25/27] Fix display of backrgound layer --- app/assets/stylesheets/components/code_listing.css.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/components/code_listing.css.scss b/app/assets/stylesheets/components/code_listing.css.scss index b0129b61f8..22e9153881 100644 --- a/app/assets/stylesheets/components/code_listing.css.scss +++ b/app/assets/stylesheets/components/code_listing.css.scss @@ -281,6 +281,7 @@ $annotation-user-background-intense: color.mix($annotation-user, $background, 20 .background-layer { position: initial; + display: block; } } From 9fb36c4e9fe787ea5c3660b3a7083f46967fcb01 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 31 Jul 2023 10:45:37 +0200 Subject: [PATCH 26/27] Correctly close html item --- .../javascripts/components/annotations/code_listing_row.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/components/annotations/code_listing_row.ts b/app/assets/javascripts/components/annotations/code_listing_row.ts index 3563b940c1..9a99ee8c73 100644 --- a/app/assets/javascripts/components/annotations/code_listing_row.ts +++ b/app/assets/javascripts/components/annotations/code_listing_row.ts @@ -77,7 +77,7 @@ export class CodeListingRow extends i18nMixin(ShadowlessLitElement) {
${this.row}
- + this.closeForm()} From 1f2812a64b29367c86cbde507a805eff883ed9cc Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 31 Jul 2023 13:29:03 +0200 Subject: [PATCH 27/27] Fix monospace font --- .../stylesheets/bootstrap_css_variable_overrides.css.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/bootstrap_css_variable_overrides.css.scss b/app/assets/stylesheets/bootstrap_css_variable_overrides.css.scss index ec1d487c7c..2b8e394edd 100644 --- a/app/assets/stylesheets/bootstrap_css_variable_overrides.css.scss +++ b/app/assets/stylesheets/bootstrap_css_variable_overrides.css.scss @@ -107,7 +107,7 @@ // font --bs-font-sans-serif: var(--d-font-sans-serif); - --bs-font-monospace: var(--d-font-family-monospace); + --bs-font-monospace: var(--d-font-monospace); --bs-gradient: linear-gradient(180deg, rgba(255, 255, 255, 0.15), rgba(255, 255, 255, 0)); --bs-body-font-family: var(--bs-font-sans-serif); --bs-body-font-size: var(--d-font-size-base);