-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Introduce editable table for creating score distributions #5723
base: main
Are you sure you want to change the base?
Changes from all commits
2ea4fd5
935ce06
0df802b
afee968
f6f7f81
e45df51
94161d2
6258c97
c1e43df
e516a08
4e8e4c9
0ddc92a
4d99358
a52eeff
2812c8d
abadbe9
2a0a4a8
40d20f9
99b27c8
61ed540
66c5247
2451a67
c68b4d0
ad5c0d8
defc89e
41d8fae
bf47ad7
973371d
a7e363d
f1ac12c
be2eec6
838e3cc
88d9bb4
370b371
cc50615
d443fa0
5cb4711
56982a5
cc80dde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,244 @@ | ||
import { customElement, property } from "lit/decorators.js"; | ||
import { html, PropertyValues, TemplateResult } from "lit"; | ||
import jspreadsheet, { Column, JspreadsheetInstance } from "jspreadsheet-ce"; | ||
import { createRef, ref, Ref } from "lit/directives/ref.js"; | ||
import { DodonaElement } from "components/meta/dodona_element"; | ||
import { fetch, ready } from "utilities"; | ||
import { i18n } from "i18n/i18n"; | ||
import { Tooltip } from "bootstrap"; | ||
|
||
type CellData = string | number | boolean; | ||
type ScoreItem = { | ||
id: number | null; | ||
name: string; | ||
description?: string; | ||
maximum: string; | ||
visible: boolean; | ||
order?: number; | ||
} | ||
|
||
type ColumnWithTooltip = Column & { tooltip?: string }; | ||
|
||
/** | ||
* A spreadsheet table to edit score items. | ||
* | ||
* @element d-score-item-input-table | ||
* | ||
* @fires cancel - When the cancel button is clicked. | ||
* | ||
* @prop {string} route - The route to send the updated score items to. | ||
* @prop {ScoreItem[]} scoreItems - The original score items, that will be displayed in the table. | ||
*/ | ||
@customElement("d-score-item-input-table") | ||
export class ScoreItemInputTable extends DodonaElement { | ||
@property({ type: String }) | ||
route: string = ""; | ||
@property({ type: Array, attribute: "score-items" }) | ||
scoreItems: ScoreItem[] = []; | ||
|
||
tableRef: Ref<HTMLDivElement> = createRef(); | ||
table: JspreadsheetInstance; | ||
|
||
@property({ state: true }) | ||
hasErrors: boolean = false; | ||
|
||
get tableWidth(): number { | ||
return this.tableRef.value.clientWidth; | ||
} | ||
|
||
get descriptionColWidth(): number { | ||
if (!this.tableRef.value) { | ||
return 200; | ||
} | ||
|
||
// full width - borders - name column - maximum column - visible column - index column | ||
const variableWidth = this.tableWidth - 14 - 200 - 100 - 100 - 50; | ||
return Math.max(200, variableWidth); | ||
} | ||
|
||
get data(): CellData[][] { | ||
return [ | ||
...this.scoreItems.map(item => [ | ||
item.id, | ||
item.name, | ||
item.description, | ||
item.maximum, | ||
item.visible | ||
]), | ||
["", "", "", "", false] | ||
]; | ||
} | ||
|
||
get editedScoreItems(): ScoreItem[] { | ||
const tableData = this.table.getData(); | ||
|
||
const scoreItems = tableData.map((row: CellData[], index: number) => { | ||
return { | ||
id: row[0] as number | null, | ||
name: row[1] as string, | ||
description: row[2] as string, | ||
maximum: (row[3] as string).replace(",", "."), // replace comma with dot for float representation | ||
visible: row[4] as boolean, | ||
order: index, | ||
}; | ||
}); | ||
|
||
// filter out empty rows | ||
return scoreItems.filter(item => !(item.name === "" && item.maximum === "" && item.description === "" && item.visible === false)); | ||
} | ||
|
||
get columnConfig(): ColumnWithTooltip[] { | ||
return [ | ||
{ type: "hidden", title: "id" }, | ||
{ type: "text", title: i18n.t("js.score_items.name"), width: 200, align: "left" }, | ||
{ type: "text", title: i18n.t("js.score_items.description"), width: this.descriptionColWidth, align: "left", tooltip: i18n.t("js.score_items.description_help") }, | ||
{ type: "numeric", title: i18n.t("js.score_items.maximum"), width: 100, align: "left", tooltip: i18n.t("js.score_items.maximum_help") }, | ||
{ type: "checkbox", title: i18n.t("js.score_items.visible"), width: 100, align: "left", tooltip: i18n.t("js.score_items.visible_help") }, | ||
]; | ||
} | ||
|
||
async initTable(): Promise<void> { | ||
// Wait for translations to be present | ||
await ready; | ||
|
||
this.table = jspreadsheet(this.tableRef.value, { | ||
root: this, | ||
data: this.data, | ||
columns: this.columnConfig, | ||
text: { | ||
copy: i18n.t("js.score_items.jspreadsheet.copy"), | ||
deleteSelectedRows: i18n.t("js.score_items.jspreadsheet.deleteSelectedRows"), | ||
insertANewRowAfter: i18n.t("js.score_items.jspreadsheet.insertNewRowAfter"), | ||
insertANewRowBefore: i18n.t("js.score_items.jspreadsheet.insertNewRowBefore"), | ||
paste: i18n.t("js.score_items.jspreadsheet.paste"), | ||
}, | ||
about: false, | ||
allowDeleteColumn: false, | ||
allowDeleteRow: true, | ||
allowInsertColumn: false, | ||
allowInsertRow: true, | ||
allowManualInsertColumn: false, | ||
allowManualInsertRow: true, | ||
allowRenameColumn: false, | ||
columnResize: false, | ||
columnSorting: false, | ||
minSpareRows: 1, | ||
parseFormulas: false, | ||
selectionCopy: false, | ||
wordWrap: true, | ||
defaultRowHeight: 30, | ||
allowExport: false, | ||
}); | ||
|
||
// init tooltips | ||
this.columnConfig.forEach((column, index) => { | ||
const td = this.tableRef.value.querySelector(`thead td[data-x="${index}"]`); | ||
if (td && column.tooltip) { | ||
td.setAttribute("title", column.tooltip); | ||
new Tooltip(td); | ||
} | ||
}); | ||
|
||
|
||
// update description column width when the window is resized | ||
new ResizeObserver(() => { | ||
this.table.setWidth(2, this.descriptionColWidth); | ||
}).observe(this.tableRef.value); | ||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
protected firstUpdated(_changedProperties: PropertyValues): void { | ||
super.firstUpdated(_changedProperties); | ||
|
||
this.initTable(); | ||
} | ||
|
||
validate(): boolean { | ||
// Remove all error classes | ||
this.tableRef.value.querySelectorAll("td.error").forEach(cell => { | ||
cell.classList.remove("error"); | ||
}); | ||
|
||
const invalidCells: string[] = []; | ||
const data = this.editedScoreItems; | ||
data.forEach(item => { | ||
const row = item.order + 1; | ||
if (item.name === "") { | ||
invalidCells.push("B" + row); | ||
} | ||
// Check if maximum is a positive number < 1000 | ||
// we use a regex instead of parseFloat because parseFloat is too lenient | ||
if (! /^\d{1,3}(\.\d*)?$/.test(item.maximum) || parseFloat(item.maximum) <= 0) { | ||
invalidCells.push("D" + row); | ||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
}); | ||
invalidCells.forEach(cell => { | ||
this.table.getCell(cell).classList.add("error"); | ||
}); | ||
this.hasErrors = invalidCells.length > 0; | ||
return !this.hasErrors; | ||
} | ||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
confirmWarnings(): boolean { | ||
const old = this.scoreItems; | ||
const edited = this.editedScoreItems; | ||
const removed = old.some(item => !edited.some(e => e.id === item.id)); | ||
const maxEdited = old.some(item => edited.some(e => e.id === item.id && e.maximum !== item.maximum)); | ||
|
||
let warnings = ""; | ||
if (removed) { | ||
warnings += i18n.t("js.score_items.deleted_warning") + "\n"; | ||
} | ||
if (maxEdited) { | ||
warnings += i18n.t("js.score_items.modified_warning") + "\n"; | ||
} | ||
|
||
return warnings === "" || confirm(warnings); | ||
} | ||
|
||
async save(): Promise<void> { | ||
if (!this.validate()) { | ||
return; | ||
} | ||
|
||
if (!this.confirmWarnings()) { | ||
return; | ||
} | ||
|
||
const response = await fetch(this.route, { | ||
method: "PATCH", | ||
headers: { | ||
"Content-Type": "application/json" | ||
}, | ||
body: JSON.stringify({ | ||
score_items: this.editedScoreItems | ||
}) | ||
}); | ||
if (response.ok) { | ||
const js = await response.text(); | ||
eval(js); | ||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+198
to
+220
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using Using Refactor the if (response.ok) {
- const js = await response.text();
- eval(js);
+ const data = await response.json();
+ // Process the data as needed
}
🧰 Tools🪛 Biome
|
||
|
||
cancel(): void { | ||
if (this.table) { | ||
this.table.setData(this.data); | ||
} | ||
this.dispatchEvent(new Event("cancel")); | ||
} | ||
|
||
|
||
render(): TemplateResult { | ||
return html` | ||
${this.hasErrors ? html`<div class="alert alert-danger">${i18n.t("js.score_items.validation_warning")}</div>` : ""} | ||
<div style="width: 100%" ${ref(this.tableRef)} contenteditable="true"></div> | ||
<div class="d-flex justify-content-end"> | ||
<button @click=${this.cancel} class="btn btn-text me-1"> | ||
${i18n.t("js.score_items.cancel")} | ||
</button> | ||
<button @click=${this.save} class="btn btn-filled"> | ||
${i18n.t("js.score_items.save")} | ||
</button> | ||
</div> | ||
`; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||
import { fetch } from "utilities"; | ||||||||||||||||||||||||||||||||||||||
import { i18n } from "i18n/i18n"; | ||||||||||||||||||||||||||||||||||||||
import { ScoreItemInputTable } from "components/input_table"; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
function commonCheckboxInit( | ||||||||||||||||||||||||||||||||||||||
element: HTMLElement, | ||||||||||||||||||||||||||||||||||||||
|
@@ -44,18 +45,28 @@ | |||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
function initItemVisibilityCheckboxes(element: HTMLElement): void { | ||||||||||||||||||||||||||||||||||||||
commonCheckboxInit(element, ".visibility-checkbox", checked => { | ||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
score_item: { | ||||||||||||||||||||||||||||||||||||||
visible: checked | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
export function initVisibilityCheckboxes(element: HTMLElement): void { | ||||||||||||||||||||||||||||||||||||||
initTotalVisibilityCheckboxes(element); | ||||||||||||||||||||||||||||||||||||||
initItemVisibilityCheckboxes(element); | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
export function initEditButton(element: HTMLElement): void { | ||||||||||||||||||||||||||||||||||||||
const editBtn = element.querySelector(".edit-btn") as HTMLAnchorElement; | ||||||||||||||||||||||||||||||||||||||
const table = element.querySelector(".score-items-table") as HTMLTableElement; | ||||||||||||||||||||||||||||||||||||||
const form = element.querySelector("d-score-item-input-table") as ScoreItemInputTable; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
const activateEditMode = (e: MouseEvent): void => { | ||||||||||||||||||||||||||||||||||||||
e.preventDefault(); | ||||||||||||||||||||||||||||||||||||||
table.classList.add("d-none"); | ||||||||||||||||||||||||||||||||||||||
editBtn.classList.add("d-none"); | ||||||||||||||||||||||||||||||||||||||
form.classList.remove("d-none"); | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
jorg-vr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
editBtn.addEventListener("click", activateEditMode); | ||||||||||||||||||||||||||||||||||||||
table.addEventListener("click", activateEditMode); | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+64
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve click target specificity and accessibility. The current implementation attaches a click handler to the entire table, which might interfere with other interactive elements within the table. Additionally, keyboard accessibility should be considered. - editBtn.addEventListener("click", activateEditMode);
- table.addEventListener("click", activateEditMode);
+ // Add role and keyboard support
+ editBtn.setAttribute("role", "button");
+ editBtn.setAttribute("tabindex", "0");
+ editBtn.addEventListener("click", activateEditMode);
+ editBtn.addEventListener("keydown", (e: KeyboardEvent) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ activateEditMode(e as unknown as MouseEvent);
+ }
+ });
+
+ // Add edit button to table header instead of entire table
+ const tableHeader = table.querySelector("thead");
+ if (tableHeader) {
+ tableHeader.addEventListener("click", activateEditMode);
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
form.addEventListener("cancel", () => { | ||||||||||||||||||||||||||||||||||||||
table.classList.remove("d-none"); | ||||||||||||||||||||||||||||||||||||||
editBtn.classList.remove("d-none"); | ||||||||||||||||||||||||||||||||||||||
form.classList.add("d-none"); | ||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dispose of
Tooltip
instances to prevent memory leaksThe
Tooltip
instances created for column headers are not disposed of when the component is disconnected. This may lead to memory leaks. Consider storing references to the tooltips and disposing of them in thedisconnectedCallback
lifecycle method.To address this, modify the code as follows:
And initialize
tooltips
in your class:export class ScoreItemInputTable extends DodonaElement { + tooltips: Tooltip[] = [];
🧰 Tools
🪛 GitHub Check: codecov/patch