Skip to content

Commit

Permalink
Merge pull request #4862 from dodona-edu/enhance/reintroduce-hover
Browse files Browse the repository at this point in the history
Reintroduce highlighting annotations on hover
  • Loading branch information
jorg-vr authored Aug 3, 2023
2 parents 6c8d4ae + 72ce85a commit b80b4f7
Show file tree
Hide file tree
Showing 13 changed files with 155 additions and 82 deletions.
42 changes: 31 additions & 11 deletions app/assets/javascripts/components/annotations/annotation_marker.ts
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);
});
}

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
12 changes: 7 additions & 5 deletions app/assets/javascripts/components/annotations/user_annotation.ts
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

0 comments on commit b80b4f7

Please sign in to comment.