Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Already on GitHub? Sign in to your account

Reintroduce highlighting annotations on hover #4862

Merged
merged 5 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { customElement, property } from "lit/decorators.js";
import { html, LitElement, TemplateResult } from "lit";
import {
AnnotationData,
annotationState,
Annotation,
compareAnnotationOrders,
isUserAnnotation
} from "state/Annotations";
import { MachineAnnotationData } from "state/MachineAnnotations";
import { MachineAnnotation } from "state/MachineAnnotations";
import { StateController } from "state/state_system/StateController";
/**
* 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.
Expand All @@ -18,7 +18,9 @@ import { MachineAnnotationData } from "state/MachineAnnotations";
@customElement("d-annotation-marker")
export class AnnotationMarker extends LitElement {
@property({ type: Array })
annotations: AnnotationData[];
annotations: Annotation[];

state = new StateController(this);

static colors = {
"error": "var(--error-color, red)",
Expand All @@ -30,30 +32,48 @@ export class AnnotationMarker extends LitElement {
"question-intense": "var(--question-intense-color, orange)",
};

static getStyle(annotation: AnnotationData): string {
static getStyle(annotation: Annotation): string {
if (["error", "warning", "info"].includes(annotation.type)) {
// shorthand notation does not work in safari
return `
text-decoration-line: underline;
text-decoration-color: ${AnnotationMarker.colors[annotation.type]};
text-decoration-thickness: 1px;
text-decoration-thickness: ${annotation.isHovered ? 2 : 1}px;
text-decoration-style: wavy;
text-decoration-skip-ink: none;
`;
} else {
const key = annotation.isHovered ? `${annotation.type}-intense` : annotation.type;
return `
background: ${AnnotationMarker.colors[annotation.type]};
background: ${AnnotationMarker.colors[key]};
`;
}
}

get sortedAnnotations(): AnnotationData[] {
return this.annotations.sort( compareAnnotationOrders );
/**
* Returns the annotations sorted in order of importance.
* Hovered annotations are prioritized over non-hovered annotations.
* Otherwise the default order is used, defined in `compareAnnotationOrders`.
*
* Goal is to always show the style of the most important annotation.
*/
get sortedAnnotations(): Annotation[] {
return this.annotations.sort( (a, b) => {
if (a.isHovered && !b.isHovered) {
return -1;
}

if (b.isHovered && !a.isHovered) {
return 1;
}

return compareAnnotationOrders(a, b);
jorg-vr marked this conversation as resolved.
Show resolved Hide resolved
});
}

get machineAnnotationMarkerSVG(): TemplateResult | undefined {
const firstMachineAnnotation = this.sortedAnnotations.find(a => !isUserAnnotation(a)) as MachineAnnotationData | undefined;
const size = 14;
const firstMachineAnnotation = this.sortedAnnotations.find(a => !isUserAnnotation(a)) as MachineAnnotation | undefined;
const size = firstMachineAnnotation?.isHovered ? 20 : 14;
return firstMachineAnnotation && html`<svg style="position: absolute; top: ${16 - size/2}px; left: -${size/2}px" width="${size}" height="${size}" viewBox="0 0 24 24">
<path fill="${AnnotationMarker.colors[firstMachineAnnotation.type]}" d="M7.41 15.41L12 10.83l4.59 4.58L18 14l-6-6l-6 6l1.41 1.41Z"/>
</svg>`;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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 { Annotation, annotationState, compareAnnotationOrders, isUserAnnotation } from "state/Annotations";
import { StateController } from "state/state_system/StateController";
import { createDelayer } from "util.js";

Expand All @@ -16,7 +16,7 @@ const setInstancesDelayer = createDelayer();
@customElement("d-annotation-tooltip")
export class AnnotationTooltip extends LitElement {
@property({ type: Array })
annotations: AnnotationData[];
annotations: Annotation[];

static styles = css`:host { position: relative; }`;

Expand Down Expand Up @@ -61,7 +61,7 @@ export class AnnotationTooltip extends LitElement {
}

// Annotations that are not displayed inline should show up as tooltips.
get hiddenAnnotations(): AnnotationData[] {
get hiddenAnnotations(): Annotation[] {
return this.annotations.filter(a => !annotationState.isVisible(a)).sort(compareAnnotationOrders);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { html, TemplateResult } from "lit";
import { UserAnnotationFormData, userAnnotationState } from "state/UserAnnotations";
import { annotationState, compareAnnotationOrders } from "state/Annotations";
import { submissionState } from "state/Submissions";
import { MachineAnnotationData, machineAnnotationState } from "state/MachineAnnotations";
import { MachineAnnotation, machineAnnotationState } from "state/MachineAnnotations";
import "components/annotations/machine_annotation";
import "components/annotations/user_annotation";
import "components/annotations/annotation_form";
Expand Down Expand Up @@ -33,7 +33,7 @@ export class AnnotationsCell extends ShadowlessLitElement {

annotationFormRef: Ref<AnnotationForm> = createRef();

get machineAnnotations(): MachineAnnotationData[] {
get machineAnnotations(): MachineAnnotation[] {
return machineAnnotationState.byLine.get(this.row) || [];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
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 { UserAnnotationData, userAnnotationState } from "state/UserAnnotations";
import { MachineAnnotation, machineAnnotationState } from "state/MachineAnnotations";
import { UserAnnotation, userAnnotationState } from "state/UserAnnotations";
import { i18nMixin } from "components/meta/i18n_mixin";
import { PropertyValues } from "@lit/reactive-element/development/reactive-element";
import { initTooltips } from "util.js";
Expand All @@ -20,15 +20,15 @@ export class HiddenAnnotationsDot extends i18nMixin(ShadowlessLitElement) {
@property({ type: Number })
row: number;

get machineAnnotations(): MachineAnnotationData[] {
get machineAnnotations(): MachineAnnotation[] {
return machineAnnotationState.byLine.get(this.row) || [];
}

get userAnnotations(): UserAnnotationData[] {
get userAnnotations(): UserAnnotation[] {
return userAnnotationState.rootIdsByLine.get(this.row)?.map(id => userAnnotationState.byId.get(id)) || [];
}

get hiddenAnnotations(): (MachineAnnotationData | UserAnnotationData)[] {
get hiddenAnnotations(): (MachineAnnotation | UserAnnotation)[] {
return [...this.machineAnnotations, ...this.userAnnotations].filter(a => !annotationState.isVisible(a));
}

Expand Down
18 changes: 9 additions & 9 deletions app/assets/javascripts/components/annotations/line_of_code.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
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 { UserAnnotationData, userAnnotationState } from "state/UserAnnotations";
import { AnnotationData, compareAnnotationOrders } from "state/Annotations";
import { MachineAnnotation, machineAnnotationState } from "state/MachineAnnotations";
import { UserAnnotation, userAnnotationState } from "state/UserAnnotations";
import { Annotation, compareAnnotationOrders } from "state/Annotations";
import { submissionState } from "state/Submissions";
import "components/annotations/annotation_marker";
import "components/annotations/annotation_tooltip";
Expand All @@ -13,7 +13,7 @@ import { unsafeHTML } from "lit/directives/unsafe-html.js";
declare type range = {
start: number;
length: number;
annotations: AnnotationData[];
annotations: Annotation[];
};

function numberArrayEquals(a: number[], b: number[]): boolean {
Expand Down Expand Up @@ -47,15 +47,15 @@ export class LineOfCode extends ShadowlessLitElement {
return this.code.length;
}

get machineAnnotationsToMark(): MachineAnnotationData[] {
get machineAnnotationsToMark(): MachineAnnotation[] {
return machineAnnotationState.byMarkedLine.get(this.row) || [];
}

get userAnnotationsToMark(): UserAnnotationData[] {
get userAnnotationsToMark(): UserAnnotation[] {
return userAnnotationState.rootIdsByMarkedLine.get(this.row)?.map(i => userAnnotationState.byId.get(i)) || [];
}

get fullLineAnnotations(): UserAnnotationData[] {
get fullLineAnnotations(): UserAnnotation[] {
return this.userAnnotationsToMark
.filter(a => !a.column && !a.columns)
.sort(compareAnnotationOrders);
Expand All @@ -67,7 +67,7 @@ export class LineOfCode 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, index: number): { start: number, length: number, index: number } {
getRangeFromAnnotation(annotation: Annotation, 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;
Expand Down Expand Up @@ -131,7 +131,7 @@ export class LineOfCode extends ShadowlessLitElement {
}

get ranges(): range[] {
const toMark: AnnotationData[] = [...this.machineAnnotationsToMark, ...this.userAnnotationsToMark];
const toMark: Annotation[] = [...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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
import { html, TemplateResult } from "lit";
import { customElement, property } from "lit/decorators.js";
import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
import { MachineAnnotationData } from "state/MachineAnnotations";
import { annotationState } from "state/Annotations";
import { MachineAnnotation } from "state/MachineAnnotations";


/**
* This component represents a machine annotation.
*
* @element d-machine-annotation
*
* @prop {MachineAnnotationData} data - The machine annotation data.
* @prop {MachineAnnotation} data - The machine annotation data.
*/
@customElement("d-machine-annotation")
export class MachineAnnotation extends ShadowlessLitElement {
export class MachineAnnotationComponent extends ShadowlessLitElement {
@property({ type: Object })
data: MachineAnnotationData;
data: MachineAnnotation;

protected get hasNotice(): boolean {
return this.data.externalUrl !== null && this.data.externalUrl !== undefined;
Expand All @@ -30,7 +29,9 @@ export class MachineAnnotation extends ShadowlessLitElement {

render(): TemplateResult {
return html`
<div class="annotation machine-annotation ${this.data.type}">
<div class="annotation machine-annotation ${this.data.type}"
@mouseenter="${() => this.data.isHovered = true}"
@mouseleave="${() => this.data.isHovered = false}">
<div class="annotation-header">
<span class="annotation-meta">
${I18n.t(`js.annotation.type.${this.data.type}`)}
Expand Down
11 changes: 6 additions & 5 deletions app/assets/javascripts/components/annotations/thread.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
import { customElement, property } from "lit/decorators.js";
import {

UserAnnotationData,
UserAnnotation,
UserAnnotationFormData, userAnnotationState
} from "state/UserAnnotations";
import { html, TemplateResult } from "lit";
Expand Down Expand Up @@ -31,11 +30,11 @@ export class Thread extends i18nMixin(ShadowlessLitElement) {

annotationFormRef: Ref<AnnotationForm> = createRef();

get data(): UserAnnotationData {
get data(): UserAnnotation {
return userAnnotationState.byId.get(this.rootId);
}

get openQuestions(): UserAnnotationData[] | undefined {
get openQuestions(): UserAnnotation[] | undefined {
return [this.data, ...this.data.responses]
.filter(response => response.question_state !== undefined && response.question_state !== "answered");
}
Expand Down Expand Up @@ -87,7 +86,9 @@ export class Thread extends i18nMixin(ShadowlessLitElement) {

render(): TemplateResult {
return this.data ? html`
<div class="thread ${annotationState.isVisible(this.data) ? "" : "hidden"}">
<div class="thread ${annotationState.isVisible(this.data) ? "" : "hidden"}"
@mouseenter="${() => this.data.isHovered = true}"
@mouseleave="${() => this.data.isHovered = false}">
<d-user-annotation .data=${this.data}></d-user-annotation>
${this.data.responses.map(response => html`
<d-user-annotation .data=${response}></d-user-annotation>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { html, PropertyValues, TemplateResult } from "lit";
import { customElement, property } from "lit/decorators.js";
import { unsafeHTML } from "lit/directives/unsafe-html.js";
import { ShadowlessLitElement } from "components/meta/shadowless_lit_element";
import { UserAnnotationData, userAnnotationState } from "state/UserAnnotations";
import { UserAnnotation, userAnnotationState } from "state/UserAnnotations";
import { i18nMixin } from "components/meta/i18n_mixin";
import { AnnotationForm } from "components/annotations/annotation_form";
import { createRef, Ref, ref } from "lit/directives/ref.js";
Expand All @@ -18,12 +18,12 @@ import { annotationState } from "state/Annotations";
*
* @element d-user-annotation
*
* @prop {UserAnnotationData} data - the data of the annotation
* @prop {UserAnnotation} data - the annotation
*/
@customElement("d-user-annotation")
export class UserAnnotation extends i18nMixin(ShadowlessLitElement) {
export class UserAnnotationComponent extends i18nMixin(ShadowlessLitElement) {
@property({ type: Object })
data: UserAnnotationData;
data: UserAnnotation;

@property({ state: true })
editing = false;
Expand Down Expand Up @@ -175,7 +175,9 @@ export class UserAnnotation extends i18nMixin(ShadowlessLitElement) {

render(): TemplateResult {
return html`
<div class="annotation ${this.data.type == "annotation" ? "user" : "question"}">
<div class="annotation ${this.data.type === "annotation" ? "user" : "question"}"
@mouseenter="${() => this.data.isHovered = true}"
@mouseleave="${() => this.data.isHovered = false}">
<div class="annotation-header">
<span class="annotation-meta">
${this.header}
Expand Down
12 changes: 6 additions & 6 deletions app/assets/javascripts/state/Annotations.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { MachineAnnotationData } from "state/MachineAnnotations";
import { AnnotationType, UserAnnotationData } from "state/UserAnnotations";
import { MachineAnnotation } from "state/MachineAnnotations";
import { AnnotationType, UserAnnotation } from "state/UserAnnotations";
import { State } from "state/state_system/State";
import { stateProperty } from "state/state_system/StateProperty";

export type AnnotationVisibilityOptions = "all" | "important" | "none";
export type AnnotationData = MachineAnnotationData | UserAnnotationData;
export type Annotation = MachineAnnotation | UserAnnotation;

const annotationOrder: Record<AnnotationType, number> = {
annotation: 0,
Expand All @@ -14,19 +14,19 @@ const annotationOrder: Record<AnnotationType, number> = {
info: 4,
};

export function compareAnnotationOrders(a: AnnotationData, b: AnnotationData): number {
export function compareAnnotationOrders(a: Annotation, b: Annotation): number {
return annotationOrder[a.type] - annotationOrder[b.type];
}

export function isUserAnnotation(annotation: AnnotationData): annotation is UserAnnotationData {
export function isUserAnnotation(annotation: Annotation): annotation is UserAnnotation {
return annotation.type === "annotation" || annotation.type === "question";
}

class AnnotationState extends State {
@stateProperty visibility: AnnotationVisibilityOptions = "all";
@stateProperty isQuestionMode = false;

isVisible(annotation: AnnotationData): boolean {
isVisible(annotation: Annotation): boolean {
if (this.visibility === "none") {
return false;
}
Expand Down
12 changes: 9 additions & 3 deletions app/assets/javascripts/state/MachineAnnotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AnnotationType } from "state/UserAnnotations";
import { State } from "state/state_system/State";
import { stateProperty } from "state/state_system/StateProperty";
import { StateMap } from "state/state_system/StateMap";
import { createStateFromInterface } from "state/state_system/CreateStateFromInterface";

export interface MachineAnnotationData {
type: AnnotationType;
Expand All @@ -13,16 +14,21 @@ export interface MachineAnnotationData {
columns?: number;
}

export class MachineAnnotation extends createStateFromInterface<MachineAnnotationData>() {
@stateProperty public isHovered = false;
}

class MachineAnnotationState extends State {
@stateProperty public byLine = new StateMap<number, MachineAnnotationData[]>();
@stateProperty public byMarkedLine = new StateMap<number, MachineAnnotationData[]>();
@stateProperty public byLine = new StateMap<number, MachineAnnotation[]>();
@stateProperty public byMarkedLine = new StateMap<number, MachineAnnotation[]>();
@stateProperty public count = 0;

public setMachineAnnotations(annotations: MachineAnnotationData[]): void {
this.count = annotations.length;
this.byLine.clear();
this.byMarkedLine.clear();
for (const annotation of annotations) {
for (const data of annotations) {
const annotation = new MachineAnnotation(data);
const markedLength = annotation.rows ?? 1;
const line = annotation.row ? annotation.row + markedLength : 1;
if (this.byLine.has(line)) {
Expand Down
Loading