diff --git a/apps/client/src/components/note_context.ts b/apps/client/src/components/note_context.ts index c63dd6f976..eade4b02ef 100644 --- a/apps/client/src/components/note_context.ts +++ b/apps/client/src/components/note_context.ts @@ -71,6 +71,19 @@ export interface NoteContextDataMap { annotations: PdfAnnotationInfo[]; scrollToAnnotation(annotationId: string, pageNumber: number): void; }; + /** Published by a board, so the right pane can list its columns and scroll the board to one. */ + boardColumns: { + /** The columns in the order the board draws them. */ + columns: { + /** What identifies the column, which is the value its cards carry. */ + value: string; + title: string; + icon: string; + /** How many cards it holds, counting what an active filter leaves in. */ + count: number; + }[]; + scrollToColumn(column: string): void; + }; saveState: { state: SaveState; }; diff --git a/apps/client/src/menus/context_menu.spec.ts b/apps/client/src/menus/context_menu.spec.ts index a6f385de02..def6d108ed 100644 --- a/apps/client/src/menus/context_menu.spec.ts +++ b/apps/client/src/menus/context_menu.spec.ts @@ -132,6 +132,44 @@ describe("contextMenu", () => { expect(menu?.querySelectorAll(".dropdown-item .use-note-color")).toHaveLength(1); }); + it("puts itself away once a submenu's parent acts, and stays up for one that only folds", async () => { + const menu = buildPage(); + const contextMenu = await buildContextMenu(); + const picked: string[] = []; + + const show = () => contextMenu.show({ + x: 10, + y: 10, + selectMenuItemHandler: (item) => { picked.push(String(item.title)); }, + items: [ + { title: "Open note", command: "openNoteInNewTab", items: [ { title: "New tab" } ] }, + { title: "More", items: [ { title: "Archived" } ] } + ] + }); + const pressRow = (title: string) => { + // Read off the row's own line: its text also holds whatever its submenu lists. + const row = [ ...menu?.querySelectorAll("li.dropdown-item") ?? [] ] + .find((item) => item.querySelector(":scope > span")?.textContent?.trim() === title); + expect(row, title).toBeTruthy(); + const press = new MouseEvent("mousedown", { bubbles: true, button: 0 }); + // happy-dom leaves the legacy `which` unset, which is what the menu reads for the + // primary button. + Object.defineProperty(press, "which", { value: 1 }); + row?.dispatchEvent(press); + }; + + await show(); + pressRow("Open note"); + expect(picked).toEqual([ "Open note" ]); + expect(contextMenu.isShown()).toBe(false); + + await show(); + pressRow("More"); + expect(picked).toEqual([ "Open note", "More" ]); + // Nothing ran, so the menu is left standing for the submenu to be reached from. + expect(contextMenu.isShown()).toBe(true); + }); + it("says whether it is up, for a host whose own press would otherwise not know", async () => { buildPage(); const contextMenu = await buildContextMenu(); diff --git a/apps/client/src/menus/context_menu.ts b/apps/client/src/menus/context_menu.ts index 05850f40e4..e5b4e0291a 100644 --- a/apps/client/src/menus/context_menu.ts +++ b/apps/client/src/menus/context_menu.ts @@ -330,7 +330,9 @@ class ContextMenu { const $link = $("") .append($icon) - .append("   ") // some space between icon and text + // An element, not spaces: in a flex row, text merges with a plain title but is + // trimmed before one boxed by `menuName()`, indenting the two kinds of row apart. + .append($("").addClass("tn-menu-gap")) .append(item.title); if ("badges" in item && item.badges) { @@ -393,8 +395,12 @@ class ContextMenu { return false; } - // Prevent submenu from failing to expand on mobile - if (!("items" in item && item.items)) { + // A submenu's parent stays open so that it can still be expanded. One carrying a + // command or handler of its own is dismissed like any other item once it has run. + const opensSubmenu = "items" in item && !!item.items; + const acts = ("handler" in item && !!item.handler) + || ("command" in item && !!item.command); + if (!opensSubmenu || acts) { this.hide(); } diff --git a/apps/client/src/menus/link_context_menu.spec.ts b/apps/client/src/menus/link_context_menu.spec.ts index a0d3d2c2ac..ca34f29fdc 100644 --- a/apps/client/src/menus/link_context_menu.spec.ts +++ b/apps/client/src/menus/link_context_menu.spec.ts @@ -92,7 +92,11 @@ describe("getItems", () => { it("folds the three places into one submenu, quick edit standing on its own", () => { const open = linkContextMenu.getOpenNoteItem(contextMenuEvent()); - expect(open).toMatchObject({ title: "link_context_menu.open_note" }); + // The entry acts as well as folding: picking it opens the note where it is opened most. + expect(open).toMatchObject({ + title: "link_context_menu.open_note", + command: "openNoteInNewTab" + }); expect("items" in open && open.items?.map((item) => "command" in item && item.command)) .toEqual([ "openNoteInNewTab", "openNoteInNewSplit", "openNoteInNewWindow" ]); expect(linkContextMenu.getQuickEditItem()).toMatchObject({ diff --git a/apps/client/src/menus/link_context_menu.ts b/apps/client/src/menus/link_context_menu.ts index e8c61320f6..1e84f46b06 100644 --- a/apps/client/src/menus/link_context_menu.ts +++ b/apps/client/src/menus/link_context_menu.ts @@ -41,12 +41,14 @@ function getQuickEditItem(): MenuItem { * The same places, folded into one submenu, for a menu that lists entries of its own beside them. * * The items keep their commands, so `handleLinkContextMenuItem` handles them from a submenu as it - * does from the top level. + * does from the top level. The entry carries the first of those commands itself, so that picking it + * opens a new tab without going into the submenu for the place it is opened in most often. */ function getOpenNoteItem(e: ContextMenuEvent | GeoMouseEvent): MenuItem { return { title: t("link_context_menu.open_note"), uiIcon: "bx bx-link-external", + command: "openNoteInNewTab", items: getOpenItems(e) }; } diff --git a/apps/client/src/stylesheets/style.css b/apps/client/src/stylesheets/style.css index 415c2f0648..4ab85522db 100644 --- a/apps/client/src/stylesheets/style.css +++ b/apps/client/src/stylesheets/style.css @@ -1702,6 +1702,12 @@ body.mobile .dropdown-submenu > .dropdown-menu { -webkit-user-select: none; } +/* What stands between a menu item's icon and its title. */ +.dropdown-item .tn-menu-gap { + flex: none; + width: 0.5em; +} + /* A name the user wrote, boxed by `menuName()`: clipped rather than widening the menu. */ .dropdown-item .tn-menu-name { display: block; diff --git a/apps/client/src/stylesheets/theme-next-dark.css b/apps/client/src/stylesheets/theme-next-dark.css index 96cdf8e3df..63fea5dce5 100644 --- a/apps/client/src/stylesheets/theme-next-dark.css +++ b/apps/client/src/stylesheets/theme-next-dark.css @@ -411,6 +411,10 @@ --board-item-hover-background-saturation: 25%; --board-item-hover-background-lightness: 35%; + --board-drop-hint-background-saturation: 20%; + --board-drop-hint-background-lightness: 45%; + --board-drop-hint-color: #ffffffc9; + --board-focus-outline-saturation: 60%; --board-focus-outline-lightness: 70%; diff --git a/apps/client/src/stylesheets/theme-next-light.css b/apps/client/src/stylesheets/theme-next-light.css index 4f1548af44..8c283ad6e7 100644 --- a/apps/client/src/stylesheets/theme-next-light.css +++ b/apps/client/src/stylesheets/theme-next-light.css @@ -410,6 +410,10 @@ --board-item-hover-background-saturation: 100%; --board-item-hover-background-lightness: 98.5%; + --board-drop-hint-background-saturation: 50%; + --board-drop-hint-background-lightness: 75%; + --board-drop-hint-color: #000000bf; + --board-focus-outline-saturation: 50%; --board-focus-outline-lightness: 34%; diff --git a/apps/client/src/translations/en/translation.json b/apps/client/src/translations/en/translation.json index 5a3ec68ac3..7172334414 100644 --- a/apps/client/src/translations/en/translation.json +++ b/apps/client/src/translations/en/translation.json @@ -739,6 +739,7 @@ "is_hidden": "Keeps a task state out of the pickers, without deleting it or the notes already in it.", "max_nesting_depth": "How many levels of children a table shows. Notes cannot be reordered while it is set.", "include_archived": "Includes archived notes in this collection, which are otherwise left out.", + "board_card_width": "For board collections, how wide the columns are drawn: narrow, medium or wide. Ignored on mobile, which draws them at a width of its own.", "enable_inbox_column": "For board collections, shows the inbox column, which lists all child notes that are not assigned to a board column.", "board_card_redirect_to": "On a board card, opening the card jumps to the note this points at instead of opening the card itself.", "sort_columns": "For board collections, the order the columns sort their cards in, which a column follows unless it is given an order of its own. Holds title, creationDate or attr: followed by a promoted attribute's name.", @@ -3428,7 +3429,7 @@ "insert-above": "Insert new above", "insert-below": "Insert new below", "move-to-top": "Move to top", - "more-columns": "More states…", + "more-columns": "More…", "delete-column": "Delete column...", "archive-column": "Archive column", "unarchive-column": "Unarchive column", @@ -3494,6 +3495,7 @@ "change-note-icon": "Change icon", "column-menu": "Column menu", "column-already-exists": "This column already exists on the board.", + "column-name-taken": "The column named “{{- column}}” already exists.", "column-definition-save-error": "The columns were saved on the board, but could not be saved to the status attribute.", "save-error": "The change could not be saved.", "hints": { @@ -3528,6 +3530,14 @@ "sort": "Sort", "sort-manually": "Manually", "default-card-order": "Default card order", + "columns-title": "Columns", + "columns-empty": "This board has no columns yet.", + "column-width": "Column width", + "drop-as-first-item": "Drop as the first item", + "drop-as-last-item": "Drop as the last item", + "column-width-narrow": "Narrow (default)", + "column-width-medium": "Medium", + "column-width-wide": "Wide", "default-card-order-description": "How cards are sorted in each column, unless you choose different sorting options for a particular column.", "sort-board-default": "Board's default", "reset-column-sorting": "Reset sorting options to default for all columns", diff --git a/apps/client/src/widgets/attribute_widgets/attr_help.ts b/apps/client/src/widgets/attribute_widgets/attr_help.ts index 75bbacbdd0..a8a07125d0 100644 --- a/apps/client/src/widgets/attribute_widgets/attr_help.ts +++ b/apps/client/src/widgets/attribute_widgets/attr_help.ts @@ -157,6 +157,7 @@ export const ATTR_HELP: AttrHelpMap = { maxNestingDepth: t("attribute_detail.max_nesting_depth"), includeArchived: t("attribute_detail.include_archived"), enableInboxColumn: t("attribute_detail.enable_inbox_column"), + boardCardWidth: t("attribute_detail.board_card_width"), sortColumns: t("attribute_detail.sort_columns"), sortColumnsDescending: t("attribute_detail.sort_columns_descending"), "calendar:view": t("attribute_detail.calendar_view"), diff --git a/apps/client/src/widgets/collections/board/api.spec.ts b/apps/client/src/widgets/collections/board/api.spec.ts index ffe8f24470..e119d7ca8c 100644 --- a/apps/client/src/widgets/collections/board/api.spec.ts +++ b/apps/client/src/widgets/collections/board/api.spec.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import appContext from "../../../components/app_context"; import FAttribute from "../../../entities/fattribute"; @@ -1883,6 +1883,62 @@ describe("renaming a column that names itself", () => { }); }); +describe("renaming a column to a name already taken", () => { + /** + * A column is identified by the name its cards carry, so writing another column's name over it + * would merge the two. The rename is refused and reported instead. + */ + it("refuses the rename, says so and leaves both columns alone", async () => { + const { api, saved } = createApi( + { columns: [ { value: "To Do" }, { value: "Done" } ] }, [ "To Do", "Done" ]); + const put = vi.spyOn(server, "put").mockResolvedValue(undefined); + const message = vi.spyOn(toast, "showMessage").mockReturnValue(undefined); + + expect(await api.setColumnTitle("To Do", "Done")).toBe(false); + + expect(message).toHaveBeenCalledWith( + "board_view.column-name-taken", undefined, "bx bx-duplicate"); + expect(put).not.toHaveBeenCalled(); + expect(saved).toEqual([]); + }); + + /** + * The name a column is known by, not only the value its cards carry: the inbox has no value + * and is named by `displayName`, so a second column under that name reads as a duplicate. + */ + it("counts a column with no cards yet, and the name the inbox goes by", async () => { + const { api, saved } = createApi( + { + columns: [ + { value: "", displayName: "Unsorted" }, + { value: "To Do" }, + { value: "Done" } + ] + }, + // "Done" is stored but holds no cards, so the board does not derive it. + [ "", "To Do" ] + ); + vi.spyOn(server, "put").mockResolvedValue(undefined); + vi.spyOn(toast, "showMessage").mockReturnValue(undefined); + + expect(await api.setColumnTitle("To Do", "Done")).toBe(false); + expect(await api.setColumnTitle("To Do", "Unsorted")).toBe(false); + expect(await api.setColumnTitle("", "Done")).toBe(false); + expect(saved).toEqual([]); + }); + + it("takes a free name, and a column keeping the one it has", async () => { + const { api } = createApi( + { columns: [ { value: "To Do" }, { value: "Done" } ] }, [ "To Do", "Done" ]); + const put = vi.spyOn(server, "put").mockResolvedValue(undefined); + + await api.setColumnTitle("To Do", "Doing"); + await api.setColumnTitle("Done", "Done"); + + expect(put).toHaveBeenCalledTimes(2); + }); +}); + describe("filing a card under the inbox", () => { /** * Landing in the inbox means carrying no value at all. A relation the card takes from elsewhere @@ -2384,3 +2440,105 @@ describe("a selection of cards", () => { expect(branches.moveAfterBranch).not.toHaveBeenCalled(); }); }); + +describe("how wide the board draws its columns", () => { + afterEach(() => vi.restoreAllMocks()); + + it("reads the label, falling back to the default for a width it does not offer", () => { + const width = (label?: string) => createApi( + {}, [], buildNote(label ? { title: "Board", "#boardCardWidth": label } + : { title: "Board" })).api.columnWidth; + + expect(width()).toBe("narrow"); + expect(width("medium")).toBe("medium"); + expect(width("wide")).toBe("wide"); + expect(width("enormous")).toBe("narrow"); + }); + + it("writes the label for a width other than the default", async () => { + const board = buildNote({ title: "Board" }); + const setLabel = vi.spyOn(attributes, "setLabel").mockResolvedValue(undefined); + const { api } = createApi({}, [], board); + + await api.setColumnWidth("wide"); + + expect(setLabel).toHaveBeenCalledWith(board.noteId, "boardCardWidth", "wide"); + }); + + /** Kept tidy: a board drawn at the default width carries no label for it at all. */ + it("takes the label off for the default width", async () => { + const board = buildNote({ title: "Board", "#boardCardWidth": "wide" }); + const setLabel = vi.spyOn(attributes, "setLabel").mockResolvedValue(undefined); + const removeLabel = vi.spyOn(attributes, "removeOwnedLabelByName") + .mockResolvedValue(true); + const { api } = createApi({}, [], board); + + await api.setColumnWidth("narrow"); + + expect(removeLabel).toHaveBeenCalledWith(board, "boardCardWidth"); + expect(setLabel).not.toHaveBeenCalled(); + }); + + /** Dropping its own label there would hand the board the inherited width straight back. */ + it("writes the default out where the board inherits another width", async () => { + const parent = buildNote({ + title: "Parent", + "#boardCardWidth(inheritable)": "wide", + children: [ { title: "Board" } ] + }); + const board = froca.getNoteFromCache(parent.getChildNoteIds()[0]); + if (!board) throw new Error("expected the board to be in froca"); + const setLabel = vi.spyOn(attributes, "setLabel").mockResolvedValue(undefined); + const removeLabel = vi.spyOn(attributes, "removeOwnedLabelByName") + .mockResolvedValue(true); + const { api } = createApi({}, [], board); + + expect(api.columnWidth).toBe("wide"); + + await api.setColumnWidth("narrow"); + + expect(setLabel).toHaveBeenCalledWith(board.noteId, "boardCardWidth", "narrow"); + expect(removeLabel).not.toHaveBeenCalled(); + }); +}); + +describe("the columns as the right pane lists them", () => { + it("names each column, gives it its icon and counts the cards it holds", () => { + const board = buildNote({ title: "Board" }); + const cards = new Map([ + [ "", [ { note: { noteId: "a" } }, { note: { noteId: "b" } } ] ], + [ "To Do", [ { note: { noteId: "c" } } ] ] + ]) as unknown as ColumnMap; + const { api } = createApi( + { + columns: [ + { value: "", displayName: "Unsorted" }, + { value: "To Do", icon: "bx bx-star" }, + { value: "Done" } + ] + }, + [ "", "To Do", "Done" ], board, "status", cards); + + expect(api.getColumnOutline([ "", "To Do", "Done" ])).toEqual([ + { value: "", title: "Unsorted", icon: "bx bxs-inbox", count: 2 }, + { value: "To Do", title: "To Do", icon: "bx bx-star", count: 1 }, + // A column the board draws no cards for still stands, and counts none. + { value: "Done", title: "Done", icon: DEFAULT_COLUMN_ICON, count: 0 } + ]); + }); + + /** A relation board keys its columns by note id, which says nothing to a reader on its own. */ + it("names a relation board's columns by the notes they point at", () => { + const target = buildNote({ title: "Alice", "#iconClass": "bx bx-user" }); + const board = buildNote({ title: "Board" }); + const { api } = createApi({}, [ "", target.noteId ], board, "~assignee"); + + expect(api.getColumnOutline([ "", target.noteId, "missing" ])).toEqual([ + { value: "", title: "board_view.inbox", icon: "bx bxs-inbox", count: 0 }, + // The note's own icon, as `FNote` gives it, class and all. + { value: target.noteId, title: "Alice", icon: "tn-icon bx bx-user", count: 0 }, + // A note the cache has not got: the value is all there is to go on. + { value: "missing", title: "missing", icon: DEFAULT_COLUMN_ICON, count: 0 } + ]); + }); +}); diff --git a/apps/client/src/widgets/collections/board/api.ts b/apps/client/src/widgets/collections/board/api.ts index fe62cb00ff..59cdc4a5dd 100644 --- a/apps/client/src/widgets/collections/board/api.ts +++ b/apps/client/src/widgets/collections/board/api.ts @@ -25,8 +25,9 @@ import { import { BoardColumnData, BoardViewData } from "."; import { currentCardTemplate, DEFAULT_CARD_TEMPLATES } from "./card_templates"; import { - type BoardStatusDefinition, canStoreColumnsInDefinition, DEFAULT_COLUMN_ICON, - DEFAULT_GROUP_BY, INBOX_COLUMN, INBOX_COLUMN_ICON + type BoardStatusDefinition, canStoreColumnsInDefinition, COLUMN_WIDTH_LABEL, type ColumnWidth, + DEFAULT_COLUMN_ICON, DEFAULT_COLUMN_WIDTH, DEFAULT_GROUP_BY, INBOX_COLUMN, INBOX_COLUMN_ICON, + parseColumnWidth } from "./columns"; import { readColumns, writeColumns } from "./column_storage"; import { ColumnItem, ColumnMap } from "./data"; @@ -561,15 +562,59 @@ export default class BoardApi { * Most columns are identified by the value their cards carry, so renaming writes that value * to every card in the column. The inbox has no value, so it stores a display name instead and * its cards are left untouched. + * + * @returns `false` when nothing was written, which keeps the caller's editor open: the name + * is blank, or another column already uses it and renaming would merge the two. */ - async setColumnTitle(column: string, title: string) { - if (!title.trim()) { - return; + setColumnTitle(column: string, title: string): false | void | Promise { + const name = title.trim(); + if (!name) { + return false; + } + + if (this.isColumnNameTaken(name, column)) { + toast.showMessage(t("board_view.column-name-taken", { column: name }), undefined, + "bx bx-duplicate"); + return false; } return column === INBOX_COLUMN - ? this.updateColumn(column, { displayName: title.trim() }) - : this.renameColumn(column, title); + ? this.updateColumn(column, { displayName: name }) + : this.renameColumn(column, name); + } + + /** + * Whether a column other than `except` already uses a name. + * + * Compares titles as well as values, since the inbox is named by `displayName`, and covers the + * stored columns so that an empty one counts. + */ + private isColumnNameTaken(name: string, except: string) { + const values = new Set([ ...this.columns, ...this.storedColumns.map(col => col.value) ]); + for (const value of values) { + if (value !== except && (value === name || this.getColumnTitle(value) === name)) { + return true; + } + } + + return false; + } + + /** + * What each column is called, the icon it shows and how many cards it holds, which the right + * pane's outline is drawn from. + * + * A relation board keys its columns by note id, so each one is named from the note's title. + */ + getColumnOutline(columns: string[]) { + return columns.map(column => ({ + value: column, + title: this.isRelationMode && column !== INBOX_COLUMN + ? froca.getNoteFromCache(column)?.title ?? column + : this.getColumnTitle(column), + icon: this.getColumnIcon(column) ?? DEFAULT_COLUMN_ICON, + count: this.byColumn?.get(column)?.length ?? 0 + })); } /** @@ -579,7 +624,9 @@ export default class BoardApi { * `NoteLink` puts in the heading — and `setColumnIcon` is not offered there. */ getColumnIcon(column: string) { - if (this.isRelationMode) { + // The inbox stands for the cards carrying no value at all, so there is no note behind it + // even on a relation board, where every other column is one. + if (this.isRelationMode && column !== INBOX_COLUMN) { return froca.getNoteFromCache(column)?.getIcon(); } @@ -629,6 +676,33 @@ export default class BoardApi { await attributes.setBooleanWithInheritance(this.parentNote, "includeArchived", shown); } + /** How wide the board draws its columns, which the board turns into a class of its own. */ + get columnWidth() { + return parseColumnWidth(this.parentNote?.getLabelValue(COLUMN_WIDTH_LABEL)); + } + + /** + * Sets how wide the columns are drawn. + * + * Picking the default removes the label, to keep the note tidy. Where a template or a parent + * sets the label, the default is written out instead: `removeOwnedLabelByName` would leave the + * inherited value in force. + */ + async setColumnWidth(width: ColumnWidth) { + const note = this.parentNote; + if (!note) return; + + const inherited = note.getAttributes("label", COLUMN_WIDTH_LABEL) + .find(attribute => attribute.noteId !== note.noteId); + if (width === DEFAULT_COLUMN_WIDTH + && parseColumnWidth(inherited?.value) === DEFAULT_COLUMN_WIDTH) { + await attributes.removeOwnedLabelByName(note, COLUMN_WIDTH_LABEL); + return; + } + + await attributes.setLabel(note.noteId, COLUMN_WIDTH_LABEL, width); + } + /** The note limit set for a column, absent if disabled. */ getColumnLimit(column: string) { return this.storedColumns.find(col => col.value === column)?.limit; diff --git a/apps/client/src/widgets/collections/board/board_drag.spec.tsx b/apps/client/src/widgets/collections/board/board_drag.spec.tsx index 9cfa974081..bebf248764 100644 --- a/apps/client/src/widgets/collections/board/board_drag.spec.tsx +++ b/apps/client/src/widgets/collections/board/board_drag.spec.tsx @@ -7,6 +7,12 @@ import { type BoardDragCallbacks, type DraggedCard, type DropPosition, useBoardDrag } from "./board_drag"; +// i18next is never initialised under test, so the overlay would be left saying nothing at all. +vi.mock("../../../services/i18n", () => ({ + t: (key: string) => key, + translationsInitializedPromise: Promise.resolve() +})); + describe("useBoardDrag, carrying a card", () => { let container: HTMLElement | undefined; let board: HTMLElement; @@ -722,10 +728,197 @@ describe("useBoardDrag, carrying a card", () => { * Two 100px columns, 200 apart, each card 50 tall. The first holds two cards, the second one. * happy-dom lays nothing out, so every box is declared. */ - function setup({ disabled = false, carried }: { + describe("carrying a card to one end of a column", () => { + /** Ten cards in the first column, scrolled so that neither end of it is on screen. */ + const tall = { + layout: [ Array.from({ length: 10 }, (_, index) => `c${index + 1}`), [ "n3" ] ], + scrollTop: 120 + }; + + /** Taken 25 below its own top edge, so the card and the pointer are never in one place. */ + function takeHold() { + press(card("c3"), 50, 65); + } + + /** + * The column runs on above and below what it shows, so the places the two ends name are + * nowhere near the ones the cards on screen give. + */ + it("places the card at the head of the column, and at its foot", () => { + setup(tall); + + takeHold(); + move(50, 225); + act(() => { vi.advanceTimersByTime(20); }); + expect(calls.move.at(-1)?.position).toEqual({ column: "To Do", index: 5 }); + + // Clear above the heading, which is the head of the column whatever it is scrolled to. + move(50, -40); + act(() => { vi.advanceTimersByTime(20); }); + expect(calls.move.at(-1)?.position).toEqual({ column: "To Do", index: 0 }); + + // Clear below the button that adds a card, which is its foot. + move(50, 440); + act(() => { vi.advanceTimersByTime(20); }); + expect(calls.move.at(-1)?.position).toEqual({ column: "To Do", index: 10 }); + }); + + /** + * Both are where a card is held to walk the column along, which is how a reader reaches a + * place in the middle of a long one. + */ + it("leaves the heading and the button that adds a card out of both ends", () => { + setup(tall); + const showing = () => !!preview()?.classList.contains("showing-drop-hint"); + const area = board.querySelector(".board-column-content"); + + takeHold(); + // On the heading, the card's own top edge standing above the column altogether. + move(50, 20); + act(() => { vi.advanceTimersByTime(100); }); + expect(showing()).toBe(false); + // What the heading is for while a card is held over it: the column walks up under it, + // and the card is placed against the cards that brings into view. + expect(area?.scrollTop).toBeLessThan(120); + + // On the button at the foot, and in the band just past it. + move(50, 360); + act(() => { vi.advanceTimersByTime(20); }); + expect(showing()).toBe(false); + expect(calls.move.at(-1)?.position).not.toEqual({ column: "To Do", index: 10 }); + + move(50, 425); + act(() => { vi.advanceTimersByTime(20); }); + expect(showing()).toBe(false); + }); + + it("says on the copy being carried what letting go would do", () => { + setup(tall); + const hint = () => preview()?.querySelector(".board-drop-hint"); + + takeHold(); + move(50, -40); + act(() => { vi.advanceTimersByTime(20); }); + expect(preview()?.classList.contains("showing-drop-hint")).toBe(true); + expect(hint()?.textContent).toBe("board_view.drop-as-first-item"); + + move(50, 440); + act(() => { vi.advanceTimersByTime(20); }); + expect(hint()?.textContent).toBe("board_view.drop-as-last-item"); + + // Back among the cards, where the card is placed against them and says nothing. + move(50, 225); + act(() => { vi.advanceTimersByTime(20); }); + expect(preview()?.classList.contains("showing-drop-hint")).toBe(false); + }); + + /** + * A card taller than the board is cut off by it, and a label in the middle of the card is + * cut off with it. The board runs 0 to 600 here and the window reaches further, so a label + * placed against the window would land below what the reader can see. + */ + it("lets the copy leave the board, but not past a sliver of itself", () => { + setup(tall); + const labelEl = () => preview() + ?.querySelector(".board-drop-hint span") as HTMLElement; + const shift = () => Number(labelEl()?.style.transform.match(/-?[\d.]+/)?.[0]); + const visible = () => + Number(preview()?.style.getPropertyValue("--board-drag-visible")); + /** How far down the page the copy has been moved, off its own transform. */ + const moved = () => + Number(preview()?.style.transform.match(/translate3d\([^,]+,\s*(-?[\d.]+)px/)?.[1]); + const showing = parseFloat(getComputedStyle(card("c3")).fontSize) * 1.5; + + // A card of its own height, carried above the board's head. Negative here: in the app + // the board's head sits below the window's. + takeHold(); + move(50, -60); + act(() => { vi.advanceTimersByTime(20); }); + // Drawn 45 tall about a middle that starts 40 below the page's top, so this is where + // its foot stands: a sliver of it inside the board's head at 0. + expect(40 + moved() + 50 / 2 + 45 / 2).toBeCloseTo(showing, 5); + release(50, -60); + + // A card taller than the board, carried past the foot but still short of leaving it. + place(card("c3"), 0, 40, 100, 2000); + press(card("c3"), 50, 65); + move(50, 440); + act(() => { vi.advanceTimersByTime(20); }); + const head = () => 40 + moved() + 2000 / 2 - 1800 / 2; + expect(head()).toBeCloseTo(515, 5); + expect(visible()).toBeCloseTo(0.034, 3); + + // Carried on out, it stops with the same sliver against the board's foot. + move(50, 1400); + act(() => { vi.advanceTimersByTime(20); }); + const stood = moved(); + expect(head()).toBeCloseTo(600 - showing, 5); + // Against the edge, where the fade has run its course. + expect(visible()).toBe(0); + + // And stands still however far past the board the pointer goes. + move(50, 1500); + act(() => { vi.advanceTimersByTime(20); }); + expect(moved()).toBe(stood); + + // The label keeps to the middle of what the board shows of the copy. + const label = 40 + moved() + 2000 / 2 + shift() * 0.9; + expect(label).toBeGreaterThan(0); + expect(label).toBeLessThan(600); + }); + + it("offers both ends of a collapsed column, which draws none of its cards", () => { + setup(); + // Collapsed before the board opened, so its cards were never drawn: the strip is its + // heading alone, and how many it holds is read off the column itself. + const strip = board.querySelectorAll(".board-column")[1]; + strip.classList.add("collapsed"); + strip.dataset.count = "7"; + strip.querySelector(".board-column-content")?.remove(); + strip.querySelector(".board-new-item")?.remove(); + place(strip, 200, 0, 40, 400); + place(strip.querySelector("h3") as HTMLElement, 200, 0, 40, 400); + + press(card("n1"), 10, 10); + move(210, 440); + act(() => { vi.advanceTimersByTime(20); }); + expect(calls.move.at(-1)?.position).toEqual({ column: "Doing", index: 7 }); + + move(210, -40); + act(() => { vi.advanceTimersByTime(20); }); + expect(calls.move.at(-1)?.position).toEqual({ column: "Doing", index: 0 }); + + // Over the strip itself it goes to the front, as it did before either end was offered. + move(210, 200); + act(() => { vi.advanceTimersByTime(20); }); + expect(calls.move.at(-1)?.position).toEqual({ column: "Doing", index: 0 }); + release(210, 200); + }); + + /** A sorted column places what it is given, so neither of its ends is on offer. */ + it("offers no end of a column that orders its own cards", () => { + setup(tall); + board.querySelectorAll(".board-column")[0].dataset.sorted = "true"; + + takeHold(); + move(50, -40); + act(() => { vi.advanceTimersByTime(20); }); + + expect(preview()?.classList.contains("showing-drop-hint")).toBe(false); + expect(calls.move.at(-1)?.position).toEqual({ column: "To Do", index: 0 }); + }); + }); + + function setup({ + disabled = false, carried, layout = [ [ "n1", "n2" ], [ "n3" ] ], scrollTop = 0 + }: { disabled?: boolean, /** The cards a press answers with, for the tests about carrying a selection. */ - carried?: string[] + carried?: string[], + /** What each column holds, for a test that needs more cards than a column can show. */ + layout?: string[][], + /** How far each column is scrolled, its cards standing that much higher on screen. */ + scrollTop?: number } = {}) { const mountPoint = document.createElement("div"); container = mountPoint; @@ -752,11 +945,11 @@ describe("useBoardDrag, carrying a card", () => { board.appendChild(Object.assign(document.createElement("div"), { className: "board-drag-layer" })); - place(board, 0, 0, 500, 400); + place(board, 0, 0, 500, 600); // Writable: the board walks itself along while something is held at its edge. Object.defineProperty(board, "scrollLeft", { value: 0, configurable: true, writable: true }); - for (const [ index, cards ] of [ [ "n1", "n2" ], [ "n3" ] ].entries()) { + for (const [ index, cards ] of layout.entries()) { const column = document.createElement("div"); column.className = "board-column"; column.dataset.column = [ "To Do", "Doing" ][index]; @@ -775,9 +968,9 @@ describe("useBoardDrag, carrying a card", () => { const area = document.createElement("div"); area.className = "board-column-content"; column.appendChild(area); - place(area, index * 200, 40, 100, 360); + place(area, index * 200, 40, 100, 300); Object.defineProperty(area, "scrollTop", { - value: 0, configurable: true, writable: true + value: scrollTop, configurable: true, writable: true }); for (const [ position, noteId ] of cards.entries()) { @@ -788,8 +981,14 @@ describe("useBoardDrag, carrying a card", () => { className: "edit-icon" })); area.appendChild(note); - place(note, index * 200, 40 + position * 60, 100, 50); + place(note, index * 200, 40 + position * 60 - scrollTop, 100, 50); } + + // The button that adds a card, which stands at the foot of every open column. + const adder = document.createElement("div"); + adder.className = "board-new-item"; + column.appendChild(adder); + place(adder, index * 200, 340, 100, 60); } // Attached from an effect, which Preact defers past the render. diff --git a/apps/client/src/widgets/collections/board/board_drag.ts b/apps/client/src/widgets/collections/board/board_drag.ts index c6a907c65b..a4060c9609 100644 --- a/apps/client/src/widgets/collections/board/board_drag.ts +++ b/apps/client/src/widgets/collections/board/board_drag.ts @@ -1,9 +1,11 @@ import { RefObject } from "preact"; import { useCallback, useEffect, useRef, useState } from "preact/hooks"; +import { t } from "../../../services/i18n"; import { useTrackedElement } from "../../react/hooks"; import { - type CardBox, cardInsertionIndex, columnAt, type ColumnBox, columnCovers, columnInsertionIndex + type CardBox, cardInsertionIndex, columnAt, type ColumnBox, columnCovers, type ColumnEnd, + columnEndAt, columnInsertionIndex } from "./drag_geometry"; import { type BoardMeasurement, measureBoard, placeInModel, toAreaY, toBoardX @@ -23,6 +25,9 @@ const TOUCH_TOLERANCE = 8; /** How much a carried card shrinks. Written with the movement, a class could not add to it. */ const DRAG_SCALE = 0.9; +/** How much of a copy put against the board's edge is left showing, in em of the card. */ +const EDGE_SHOWING = 1.5; + /** * How many cards a carried selection is drawn as, however many are on the move: the one holding the * count and the two behind it. `index.css` draws the ones behind from `data-layers`. @@ -199,7 +204,10 @@ export function useBoardDrag( }); } held.preview = lifted.preview; + held.hint = lifted.hint; held.grab = lifted.grab; + held.ownHue = lifted.ownHue; + held.showing = lifted.showing; container.classList.add("board-dragging"); setDragging(true); @@ -237,21 +245,29 @@ export function useBoardDrag( const column = columnAt(measurement.columns, x); const area = column && measurement.areas.get(column.value); + // A pointer past either end places the card at that end whatever the column is + // scrolled to, which saves dragging it the length of a long column. Read from the + // pointer rather than the card's top edge, as auto-scrolling is: both ends lie outside + // the column, and a card held below its top edge cannot reach past the button. + const end = column ? columnEndAt(column, held.lastY) : undefined; const edges: ScrollTarget[] = [ { element: container, axis: "x" } ]; - if (area) { + // No vertical auto-scroll while an end is the target: the index is already decided, + // and scrolling would suggest the card is being placed where it passes. + if (area && !end) { edges.push({ element: area, axis: "y" }); } scroller.update(edges, held.lastX, held.lastY); + markEnd(held, end, column); - // A collapsed column draws no card area and holds no cards on screen, so a card carried - // over it goes to the front of whatever it holds. + // A collapsed column draws no card area, so a card carried over it goes to the front. + // Either end can still be asked for, placing a card there without opening the column. const position = column ? { column: column.value, - index: area + index: endIndex(end, column) ?? (area ? placeAt(area, toAreaY(area, topY), held.card, column) - : 0 + : 0) } : null; // Standing on the column itself, which for a collapsed one is its heading alone: the @@ -274,7 +290,86 @@ export function useBoardDrag( report(held, position); }; + /** + * Shows the overlay on the carried copy, naming the end of the column the card would go to, + * and hides it again for a position among the cards. + * + * Paints it in the card's own colour, or in the hue of `over` where the card has none. + * + * Shifts the label towards the part of the copy still inside the board, so that a card + * taller than the board keeps it visible, and fades the copy by how much of it the board + * no longer shows. `grab` and `viewport` were both measured when the card was picked up, so + * this reads no layout. + */ + const markEnd = (held: Gesture, end: ColumnEnd | undefined, over?: ColumnBox) => { + const hint = held.hint; + const grab = held.grab; + const view = held.measurement?.viewport; + if (!hint || !grab || !view) return; + + const label = hint.firstElementChild; + if (end !== held.end) { + held.end = end; + held.preview?.classList.toggle("showing-drop-hint", !!end); + if (end && label instanceof HTMLElement) { + label.textContent = t(end === "first" + ? "board_view.drop-as-first-item" + : "board_view.drop-as-last-item"); + // Read once, as the label is written: every move that follows works from + // this rather than reading the page again. + held.labelHeight = label.offsetHeight; + } + } + + if (!end || !(label instanceof HTMLElement)) return; + + const hue = held.ownHue || over?.hue; + held.preview?.classList.toggle("hint-tinted", !!hue); + if (hue) { + held.preview?.style.setProperty("--board-drop-hint-hue", hue); + } + + const middle = held.lastY - grab.y + grab.height / 2 + holdBack(held); + const drawn = grab.height * DRAG_SCALE / 2; + const top = Math.max(middle - drawn, view.top); + const bottom = Math.min(middle + drawn, view.bottom); + // How much of the copy is in view, measured against the least it can show rather than + // against nothing: 1 while wholly in view, 0 once it is against the edge. + const height = drawn * 2; + const least = height > 0 ? Math.min(1, (held.showing ?? 0) / height) : 1; + const shown = height > 0 ? Math.max(0, Math.min(1, (bottom - top) / height)) : 1; + const visible = least < 1 ? Math.max(0, (shown - least) / (1 - least)) : 1; + held.preview?.style.setProperty("--board-drag-visible", visible.toFixed(3)); + + // Outside the board altogether: there is no part of the copy to keep the label in. + const shift = bottom > top ? ((top + bottom) / 2 - middle) / DRAG_SCALE : 0; + // The label stands on the card, so it goes no further than the card's own edges. + const room = Math.max(0, (grab.height - (held.labelHeight ?? 0)) / 2); + label.style.transform = + `translateY(${Math.max(-room, Math.min(room, shift))}px)`; + }; + /** Says where the card would land, and whether it has come to rest on that column. */ + /** + * How far back from the pointer the copy is drawn while an end is being asked for, so that + * it leaves the board only until {@link EDGE_SHOWING} of it is still in view. Returns 0 + * while no end is being asked for, where the copy follows the pointer exactly. + */ + const holdBack = (held: Gesture) => { + const grab = held.grab; + const view = held.measurement?.viewport; + if (!held.end || !grab || !view) return 0; + + const middle = held.lastY - grab.y + grab.height / 2; + const drawn = grab.height * DRAG_SCALE / 2; + const inset = held.showing ?? 0; + const wanted = held.end === "first" + ? Math.max(middle, view.top + inset - drawn) + : Math.min(middle, view.bottom - inset + drawn); + + return wanted - middle; + }; + const report = (held: Gesture, next?: DropPosition | null) => { if (held.kind !== "card") return; @@ -358,11 +453,13 @@ export function useBoardDrag( held.frame = undefined; if (!gesture.current || !held.preview) return; + // Resolved first: where the copy stands depends on the end being asked for. + resolve(held); + const dx = held.lastX - held.startX; - const dy = held.lastY - held.startY; + const dy = held.lastY - held.startY + holdBack(held); const scale = held.kind === "card" ? ` scale(${DRAG_SCALE})` : ""; held.preview.style.transform = `translate3d(${dx}px, ${dy}px, 0)${scale}`; - resolve(held); }); } }; @@ -589,6 +686,20 @@ function placeIn(cards: CardBox[], y: number, card: DraggedCard, column: string) return index >= card.index ? index + 1 : index; } +/** + * The place one end of a column names, or nothing where neither end is being asked for. + * + * The cards were counted with the carried one among them, which is the list a move is expressed + * in, so the last place is that count rather than one less. + */ +function endIndex(end: ColumnEnd | undefined, column: ColumnBox) { + if (!end) { + return undefined; + } + + return end === "first" ? 0 : column.count; +} + /** Asks an element for its menu the way a right click does, at the place the tap landed. */ function askForMenu(element: HTMLElement, clientX: number, clientY: number) { element.dispatchEvent(new MouseEvent("contextmenu", { bubbles: true, clientX, clientY })); @@ -691,6 +802,17 @@ function lift(held: Gesture, container: HTMLElement) { preview.querySelector(".edit-icon")?.remove(); preview.querySelector(".board-new-item")?.remove(); + // The overlay saying the card would go to one end of a column, hidden until it would. Built + // with the copy so a move only turns it on, and only for a card: a column is not placed this + // way. + let hint: HTMLElement | undefined; + if (held.kind === "card") { + hint = document.createElement("div"); + hint.className = "board-drop-hint"; + hint.appendChild(document.createElement("span")); + preview.appendChild(hint); + } + // The copy is carried in the drag layer rather than in the column it came from, where the rule // that tints a card no longer reaches it, so the column's hue is put on the copy itself. const hue = held.element.closest(".board-column.with-hue") @@ -700,6 +822,14 @@ function lift(held: Gesture, container: HTMLElement) { preview.style.setProperty("--board-column-custom-hue", hue); } + // The card's own colour and its em, read here rather than on every move: both cost a page + // read. + const style = getComputedStyle(held.element); + const ownHue = held.element.classList.contains("with-hue") + ? style.getPropertyValue("--custom-color-hue").trim() + : undefined; + const showing = (parseFloat(style.fontSize) || 0) * EDGE_SHOWING; + for (const [ property, value ] of Object.entries({ left: `${rect.left}px`, top: `${rect.top}px`, @@ -717,6 +847,9 @@ function lift(held: Gesture, container: HTMLElement) { held.element.style.display = "none"; return { preview, + hint, + ownHue, + showing, size: { width: rect.width, height: rect.height }, // Where inside it the reader took hold, so the middle can be found from the pointer. grab: { @@ -782,6 +915,16 @@ type Gesture = (CardSubject | ColumnSubject) & { active: boolean; /** The copy that follows the pointer, what it stands for staying where it is. */ preview?: HTMLElement; + /** The overlay on the copy saying the card would go to one end of a column. */ + hint?: HTMLElement; + /** Which end that is, so the overlay is written only as it changes. */ + end?: ColumnEnd; + /** The carried card's own colour as a hue, which the overlay takes before a column's. */ + ownHue?: string; + /** What the label inside it measures, read when it is written rather than on every move. */ + labelHeight?: number; + /** How much of the copy is left in view once it stands against the board's edge. */ + showing?: number; /** Where the press landed inside it, and how big it is, for finding its middle. */ grab?: { x: number, y: number, width: number, height: number }; measurement?: BoardMeasurement; diff --git a/apps/client/src/widgets/collections/board/column.tsx b/apps/client/src/widgets/collections/board/column.tsx index 399475fd6b..57f692880a 100644 --- a/apps/client/src/widgets/collections/board/column.tsx +++ b/apps/client/src/widgets/collections/board/column.tsx @@ -596,6 +596,11 @@ export default function Column({ return (
{ .toEqual([ "", "To Do" ]); }); }); + +describe("parseColumnWidth", () => { + /** The label is the user's to write by hand, so it can name a width the board does not have. */ + it("reads the three widths and falls back to the narrow default", () => { + expect(parseColumnWidth("narrow")).toBe("narrow"); + expect(parseColumnWidth("medium")).toBe("medium"); + expect(parseColumnWidth("wide")).toBe("wide"); + + expect(parseColumnWidth("enormous")).toBe("narrow"); + expect(parseColumnWidth("")).toBe("narrow"); + expect(parseColumnWidth(null)).toBe("narrow"); + expect(parseColumnWidth(undefined)).toBe("narrow"); + }); +}); + +describe("columnWidthClass", () => { + it("names a class only for a width the board names itself", () => { + expect(columnWidthClass("narrow")).toBe("board-narrow-columns"); + expect(columnWidthClass("medium")).toBe("board-medium-columns"); + expect(columnWidthClass("wide")).toBe("board-wide-columns"); + + // Nothing to wear, so `--board-column-width` keeps the value it inherits. A theme setting + // that variable is what this leaves room for. + expect(columnWidthClass("enormous")).toBeUndefined(); + expect(columnWidthClass("")).toBeUndefined(); + expect(columnWidthClass(null)).toBeUndefined(); + expect(columnWidthClass(undefined)).toBeUndefined(); + }); +}); diff --git a/apps/client/src/widgets/collections/board/columns.ts b/apps/client/src/widgets/collections/board/columns.ts index 3604ed442f..2dd338fac6 100644 --- a/apps/client/src/widgets/collections/board/columns.ts +++ b/apps/client/src/widgets/collections/board/columns.ts @@ -32,6 +32,37 @@ export const INBOX_COLUMN = ""; /** Default icon for the inbox column, used instead of the standard one until another is picked. */ export const INBOX_COLUMN_ICON = "bx bxs-inbox"; +/** The label naming how wide the board draws its columns. */ +export const COLUMN_WIDTH_LABEL = "boardCardWidth"; + +/** The widths the board offers, in the order the properties dialog lists them. */ +export const COLUMN_WIDTHS = [ "narrow", "medium", "wide" ] as const; + +export type ColumnWidth = typeof COLUMN_WIDTHS[number]; + +/** The width a board with no label of its own draws its columns at. */ +export const DEFAULT_COLUMN_WIDTH: ColumnWidth = "narrow"; + +/** + * Reads a stored width, falling back to the default for anything the board does not offer. The + * label is the user's to edit by hand, so it can name a width that does not exist. + */ +export function parseColumnWidth(value: string | null | undefined) { + return COLUMN_WIDTHS.find(width => width === value) ?? DEFAULT_COLUMN_WIDTH; +} + +/** + * The class a board wears for the width it names, or nothing where it names none. + * + * A board that names no width wears no class, so `--board-column-width` keeps whatever value it + * inherits: the default in this stylesheet, or a theme's own if one sets it. + */ +export function columnWidthClass(value: string | null | undefined) { + const width = COLUMN_WIDTHS.find(candidate => candidate === value); + + return width ? `board-${width}-columns` : undefined; +} + export interface BoardStatusDefinition { /** The definition attribute, wherever it is owned. */ attribute: FAttribute; diff --git a/apps/client/src/widgets/collections/board/drag_geometry.spec.ts b/apps/client/src/widgets/collections/board/drag_geometry.spec.ts index 534ef7a9f4..8adb61f8e2 100644 --- a/apps/client/src/widgets/collections/board/drag_geometry.spec.ts +++ b/apps/client/src/widgets/collections/board/drag_geometry.spec.ts @@ -1,11 +1,14 @@ import { describe, expect, it } from "vitest"; import { - type CardBox, cardInsertionIndex, columnAt, type ColumnBox, columnCovers, + type CardBox, cardInsertionIndex, columnAt, type ColumnBox, columnCovers, columnEndAt, columnInsertionIndex, columnStandsAside, movesColumn } from "./drag_geometry"; -/** Three 100px columns with a 20px gap, standing 400 tall from the top of the page. */ +/** + * Three 100px columns with a 20px gap, standing 400 tall from the top of the page. The heading + * begins at the top of each and the button that adds a card ends at its foot. + */ function columns(cards: Record = {}): ColumnBox[] { return [ "To Do", "Doing", "Done" ].map((value, index) => ({ value, @@ -14,7 +17,11 @@ function columns(cards: Record = {}): ColumnBox[] { top: 0, origin: 0, height: 400, - cards: cards[value] ?? [] + headStart: 0, + footEnd: 400, + sorted: false, + cards: cards[value] ?? [], + count: (cards[value] ?? []).length })); } @@ -80,7 +87,8 @@ describe("columnCovers", () => { */ it("tells the column from the empty space below a short one", () => { const strip: ColumnBox = { - value: "Parked", left: 0, width: 36, top: 0, height: 90, origin: 0, cards: [] + value: "Parked", left: 0, width: 36, top: 0, height: 90, origin: 0, + headStart: 90, footEnd: 90, sorted: false, cards: [], count: 0 }; expect(columnCovers(strip, 18, 0)).toBe(true); @@ -203,3 +211,45 @@ describe("columnStandsAside", () => { .toEqual([ 0, 0, 0, -266, -266, -266 ]); }); }); + +describe("columnEndAt", () => { + /** + * The two ends stand 30px clear of the column, the heading and the add button being where a + * card is held to walk the column along rather than to place it. + */ + it("answers for what stands 30px above the heading and below the add button", () => { + const [ column ] = columns(); + + expect(columnEndAt(column, -31)).toBe("first"); + expect(columnEndAt(column, -200)).toBe("first"); + + // The heading itself, the button at the foot, and the band past each of them. + expect(columnEndAt(column, -30)).toBeUndefined(); + expect(columnEndAt(column, 0)).toBeUndefined(); + expect(columnEndAt(column, 200)).toBeUndefined(); + expect(columnEndAt(column, 400)).toBeUndefined(); + expect(columnEndAt(column, 429)).toBeUndefined(); + + expect(columnEndAt(column, 430)).toBe("last"); + expect(columnEndAt(column, 900)).toBe("last"); + }); + + /** A sorted column places what it is given, so neither of its ends is the reader's to pick. */ + it("offers neither end of a column that orders its own cards", () => { + const [ column ] = columns(); + const sorted = { ...column, sorted: true }; + + expect(columnEndAt(sorted, 0)).toBeUndefined(); + expect(columnEndAt(sorted, 900)).toBeUndefined(); + }); + + /** A collapsed column is a heading and nothing else, and so is neither end of itself. */ + it("offers neither end of a column with no room between them", () => { + const [ column ] = columns(); + const strip = { ...column, headStart: 40, footEnd: 40 }; + + expect(columnEndAt(strip, 0)).toBeUndefined(); + expect(columnEndAt(strip, 40)).toBeUndefined(); + expect(columnEndAt(strip, 900)).toBeUndefined(); + }); +}); diff --git a/apps/client/src/widgets/collections/board/drag_geometry.ts b/apps/client/src/widgets/collections/board/drag_geometry.ts index e892092c36..f4d0e6f082 100644 --- a/apps/client/src/widgets/collections/board/drag_geometry.ts +++ b/apps/client/src/widgets/collections/board/drag_geometry.ts @@ -35,11 +35,64 @@ export interface ColumnBox { * boundary falls into the slot below the one the gap is drawn at. */ origin: number; + /** + * Where the heading begins and where the button that adds a card ends, down the page. A pointer + * past either one places the card at that end of the column. Neither the heading nor the button + * counts as past: over them the column auto-scrolls instead, which is how a position in the + * middle is reached. These hold for the length of a drag for the same reason {@link top} does. + */ + headStart: number; + footEnd: number; + /** Whether the column sorts its own cards, in which case the drop position is not chosen. */ + sorted: boolean; + /** The column's own colour as a hue, or nothing where it has none. */ + hue?: string; /** * The cards as drawn, in order, the dragged one included. Counting it keeps the index in the * same terms as the list the board holds, which is what a move is expressed in. */ cards: CardBox[]; + /** + * How many cards the column holds, the dragged one included. Read off the column rather than + * counted here: a collapsed one draws none of them, and a windowed one only a slice. + */ + count: number; +} + +/** Which end of a column a card would be placed at. */ +export type ColumnEnd = "first" | "last"; + +/** + * How far past the heading or the button the pointer must go for that end to count, in pixels. + * + * Between the two the column auto-scrolls, and a pointer that has only just left that band is + * usually crossing it rather than aiming at an end. + */ +const END_OFFSET = 30; + +/** + * The end of a column a point lies past: above the heading, or below the button that adds a card, + * by {@link END_OFFSET} in either case. + * + * A card dropped there goes to that end whatever the column is scrolled to, which saves dragging it + * the length of a long column. Returns `undefined` for a sorted column, where the drop position is + * not chosen. + * + * @param y the pointer, down the page. A position among the cards is read from the carried card's + * top edge instead; both ends lie outside the column, which only the pointer reaches. + */ +export function columnEndAt(column: ColumnBox, y: number): ColumnEnd | undefined { + // With no space between the two ends there is no middle, and every point would match one of + // them. + if (column.sorted || column.footEnd <= column.headStart) { + return undefined; + } + + if (y < column.headStart - END_OFFSET) { + return "first"; + } + + return y >= column.footEnd + END_OFFSET ? "last" : undefined; } /** diff --git a/apps/client/src/widgets/collections/board/drag_measure.spec.ts b/apps/client/src/widgets/collections/board/drag_measure.spec.ts index 7c4f861563..8952ef012c 100644 --- a/apps/client/src/widgets/collections/board/drag_measure.spec.ts +++ b/apps/client/src/widgets/collections/board/drag_measure.spec.ts @@ -35,6 +35,12 @@ function buildBoard({ scrollLeft = 200, areaScrollTop = 20, cardCounts = [ 2, 1 // On screen the columns have already been scrolled 200px to the left. place(column, { left: 50 + index * 120 - scrollLeft, top: 0, width: 100, height: 400 }); + // The column is padded, so neither its heading nor the button at its foot is flush with + // it: the two ends are read from those rather than from the column's own box. + const heading = document.createElement("h3"); + column.appendChild(heading); + place(heading, { left: 0, top: 8, width: 100, height: 40 }); + const area = document.createElement("div"); area.className = "board-column-content"; column.appendChild(area); @@ -57,6 +63,13 @@ function buildBoard({ scrollLeft = 200, areaScrollTop = 20, cardCounts = [ 2, 1 } } + for (const column of container.querySelectorAll(".board-column")) { + const adder = document.createElement("div"); + adder.className = "board-new-item"; + column.appendChild(adder); + place(adder, { left: 0, top: 352, width: 100, height: 40 }); + } + return container; } @@ -394,3 +407,39 @@ describe("measuring a windowed column", () => { expect(column.cards).toHaveLength(60); }); }); + +describe("what a column offers a card carried past it", () => { + it("reads the two ends from the heading and the button at the foot", () => { + const [ first ] = measureBoard(buildBoard()).columns; + + // The column's own box runs 0 to 400; these are its heading and its button. + expect(first.headStart).toBe(8); + expect(first.footEnd).toBe(392); + expect(first.sorted).toBe(false); + }); + + it("reads a column that orders its own cards off the column itself", () => { + const board = buildBoard(); + board.querySelectorAll(".board-column")[1].dataset.sorted = "true"; + + expect(measureBoard(board).columns.map((column) => column.sorted)).toEqual([ false, true ]); + }); +}); + +describe("what the reader sees of the board", () => { + it("is the note list holding it, and the board itself where it stands outside one", () => { + const board = buildBoard(); + + expect(measureBoard(board).viewport).toEqual({ top: 0, bottom: 400 }); + + // The note list is what scrolls and clips, so a board inside one is seen through it. + const list = document.createElement("div"); + list.className = "note-list-widget-content"; + document.body.appendChild(list); + place(list, { left: 0, top: 100, width: 600, height: 250 }); + list.appendChild(board); + + expect(measureBoard(board).viewport).toEqual({ top: 100, bottom: 350 }); + list.remove(); + }); +}); diff --git a/apps/client/src/widgets/collections/board/drag_measure.ts b/apps/client/src/widgets/collections/board/drag_measure.ts index 65a53c2b10..47b0e3ab7e 100644 --- a/apps/client/src/widgets/collections/board/drag_measure.ts +++ b/apps/client/src/widgets/collections/board/drag_measure.ts @@ -7,6 +7,12 @@ export interface BoardMeasurement { columns: ColumnBox[]; /** Each column's card area, which a point has to be read against to place a card in it. */ areas: Map; + /** + * What the reader sees of the board, down the page: the box of the note list holding it, or + * the board itself where it stands outside one. The copy being carried is clipped to this + * rather than to the window, so it is what says how much of the copy can be seen. + */ + viewport: { top: number, bottom: number }; } /** @@ -20,7 +26,10 @@ export interface BoardMeasurement { * pass over the whole board that nothing goes on to look at. */ export function measureBoard(container: HTMLElement, withCards = true): BoardMeasurement { - const origin = container.getBoundingClientRect().left - container.scrollLeft; + const box = container.getBoundingClientRect(); + const origin = box.left - container.scrollLeft; + const clipped = (container.closest(".note-list-widget-content") ?? container) + .getBoundingClientRect(); const columns: ColumnBox[] = []; const areas = new Map(); @@ -41,18 +50,31 @@ export function measureBoard(container: HTMLElement, withCards = true): BoardMea areas.set(value, area); } + const heading = element.querySelector(":scope > h3"); + const adder = element.querySelector(":scope > .board-new-item"); + + const cards = withCards ? measureCards(area) : []; + const stated = Number(element.dataset.count); + columns.push({ value, left: rect.left - origin, width: rect.width, top: rect.top, height: rect.height, + headStart: heading?.getBoundingClientRect().top ?? rect.top, + footEnd: adder?.getBoundingClientRect().bottom ?? rect.bottom, + sorted: element.dataset.sorted === "true", + hue: element.classList.contains("with-hue") + ? element.style.getPropertyValue("--board-column-custom-hue") + : undefined, origin: withCards ? contentOrigin(area) : 0, - cards: withCards ? measureCards(area) : [] + cards, + count: Number.isFinite(stated) ? stated : cards.length }); } - return { columns, areas }; + return { columns, areas, viewport: { top: clipped.top, bottom: clipped.bottom } }; } /** diff --git a/apps/client/src/widgets/collections/board/index.css b/apps/client/src/widgets/collections/board/index.css index e7ad197c68..ed1fd42926 100644 --- a/apps/client/src/widgets/collections/board/index.css +++ b/apps/client/src/widgets/collections/board/index.css @@ -16,7 +16,12 @@ /* Matches the height a normal header takes, so a collapsed column reads as one turned on its side. */ --board-collapsed-column-width: 2.25em; - --board-column-width: 275px; + /* The three widths the board properties offer, and the one in use. Each is its own variable so + a theme can change one width without taking over which of them a board picks. */ + --board-column-width-narrow: 275px; + --board-column-width-medium: 325px; + --board-column-width-wide: 400px; + --board-column-width: var(--board-column-width-narrow); --board-column-border-width: 2px; --board-collapse-duration: 1s; --board-collapse-duration-quick: 0.3s; @@ -27,12 +32,21 @@ --board-item-hover-shadow: 2px 4px 8px rgba(0, 0, 0, 0.1); /* What a carried card casts, which the stack of a carried selection is drawn behind. */ --board-drag-shadow: 4px 8px 16px rgba(0, 0, 0, 0.5); + /* What a carried copy is drawn at, and what it fades to once it stands against the board's + edge with only a sliver of itself in view. */ + --board-drag-opacity: 0.85; + --board-drag-opacity-faded: 0.4; /* How far each card of a carried stack stands out from the one in front of it. */ --board-drag-stack-offset: 4px; /* The ring a carried card wears, which the stack behind it starts clear of. */ --board-drag-ring-width: 3px; /* The ring a picked-out card carries. */ --board-selected-ring-color: var(--link-color); + /* The overlay on a carried card while it would drop at one end of a column, painted in the hue + the card itself is. */ + --board-drop-hint-background-saturation: 35%; + --board-drop-hint-background-lightness: 25%; + --board-drop-hint-color: #ffffff; /* Dims the board while a card is being named. Kept light: it draws the eye to the field rather than hiding what is behind it. */ --board-edit-backdrop: rgba(0, 0, 0, 0.15); @@ -110,6 +124,30 @@ body.mobile .board-view-container.board-dragging { scroll-behavior: auto; } +/* While something is carried, everything the pointer crosses says so: the copy following it is out + of hit testing, so the cursor comes from whatever lies beneath. It outweighs the cursors the + heading, the cards and the buttons set for themselves, and the class goes on once a gesture + rather than once a step, which is what makes a rule reaching this far affordable. */ +.board-view-container.board-dragging, +.board-view-container.board-dragging * { + cursor: grabbing !important; +} + +/* Set from `#boardCardWidth`, and only while the board carries it: without one of these classes + `--board-column-width` keeps the value it inherits, which a theme can set. The columns and + everything measured against them read the width off that one variable. */ +.board-view.board-narrow-columns { + --board-column-width: var(--board-column-width-narrow); +} + +.board-view.board-medium-columns { + --board-column-width: var(--board-column-width-medium); +} + +.board-view.board-wide-columns { + --board-column-width: var(--board-column-width-wide); +} + .board-view-container .board-column { width: var(--board-column-width); flex-shrink: 0; @@ -147,7 +185,6 @@ body.mobile .board-view-container .board-add-column { .board-view-container .board-column.drag-over { border-color: var(--main-text-color); - background-color: var(--hover-item-background-color); } .board-view-container .board-column h3 { @@ -181,8 +218,11 @@ body.mobile .board-view-container .board-add-column { .board-view-container .board-column h3 > .column-icon { /* Sized down from the note header's button, whose scale suits a toolbar rather than a heading. */ - --note-icon-size: 1.15em; - --note-icon-container-padding-size: 0.15em; + --note-icon-size: 1em; + /* In pixels, as everywhere else the ring is set: it has to be whole for the glyph to sit centred + in the disc, and the glyph alone follows the heading's font. The two together give the disc + its width. */ + --note-icon-container-padding-size: 5px; flex-shrink: 0; margin-inline-end: 0.4em; @@ -416,15 +456,19 @@ body.mobile .board-view-container .board-add-column { /* * The title and the icon before it, which stand where the editor's field and picker do: the icon at - * `--card-icon-inset` from the card's edge and centred on the first line, the title at - * `--card-title-inset` with the lines below it kept off the icon, as the field's own padding keeps - * them. + * `--card-icon-inset` from the card's edge, the title at `--card-title-inset` with the lines below + * it kept off the icon, as the field's own padding keeps them. + * + * A hanging indent rather than two boxes beside each other: the icon shares the first line's box, + * so it and the title rest on one baseline and rasterise on the same pixel row. Laid out as boxes + * of their own they round to the grid apart, which put the icon a pixel off on some cards. * * The card's padding is already in front of both, so each is drawn back by it. */ .board-view-container .board-note > .title { - display: flex; - align-items: flex-start; + display: block; + padding-inline-start: calc(var(--card-title-inset) - var(--card-padding)); + text-indent: calc(var(--card-icon-inset) - var(--card-title-inset)); /* The spans mark.js wraps a filter's matched tokens in. Matches the search result cards. */ .ck-find-result { @@ -441,16 +485,16 @@ body.mobile .board-view-container .board-add-column { text-decoration: underline; } +/* Inline, so the glyph sits on the title's own baseline. The trailing space assumes the pack draws + its icons on a `--card-icon-size` advance, as the layout above already did. */ .board-view-container .board-note > .title > .icon { - flex: none; - display: flex; - align-items: center; - justify-content: center; - width: var(--card-icon-size); - height: calc(var(--card-line-height) * 1em); - margin-inline-start: calc(var(--card-icon-inset) - var(--card-padding)); + display: inline; margin-inline-end: calc( var(--card-title-inset) - var(--card-icon-inset) - var(--card-icon-size)); + /* A pack centres its ink on the em box, which sits above the middle of the title's capitals. In + whole pixels, so the glyph keeps a fixed relation to the row the line is drawn on. */ + vertical-align: -1px; + vertical-align: round(-0.0555em, 1px); } .board-view-container .board-note > .edit-icon { @@ -555,11 +599,55 @@ body.mobile .board-view-container .board-add-column { will-change: transform; z-index: 1000; pointer-events: none; - cursor: move; - opacity: 0.85; + opacity: var(--board-drag-opacity); box-shadow: var(--board-drag-shadow); } +/* Opaque rather than tinted: while it is up, the message replaces the card's own content. */ +.board-view-container .board-drag-preview .board-drop-hint { + display: none; + position: absolute; + inset: 0; + align-items: center; + justify-content: center; + border-radius: inherit; + padding: var(--card-padding); + /* No saturation until `hint-tinted` sets it, so an unnamed hue paints grey rather than red. */ + background-color: hsl( + var(--board-drop-hint-hue, 0), + 0%, + var(--board-drop-hint-background-lightness) + ); + text-align: center; + font-weight: 600; + color: var(--board-drop-hint-color); + transition: background-color 250ms ease, color 250ms ease; +} + +/* The card's own hue, or the hue of the column it is held over. */ +.board-view-container .board-drag-preview.hint-tinted .board-drop-hint { + background-color: hsl( + var(--board-drop-hint-hue), + var(--board-drop-hint-background-saturation), + var(--board-drop-hint-background-lightness) + ); +} + +/* No focus ring while the overlay is up: it would outline the overlay rather than the card. The + opacity follows `--board-drag-visible`, 1 while the copy is in view and 0 against the edge. */ +.board-view-container .board-drag-preview.showing-drop-hint { + outline: none; + opacity: calc( + var(--board-drag-opacity-faded) + + (var(--board-drag-opacity) - var(--board-drag-opacity-faded)) + * var(--board-drag-visible, 1) + ); +} + +.board-view-container .board-drag-preview.showing-drop-hint .board-drop-hint { + display: flex; +} + /* What a carried selection is drawn as: one card-shaped block holding how many are on the move. */ .board-view-container .board-note.board-drag-count { display: flex; diff --git a/apps/client/src/widgets/collections/board/index.spec.tsx b/apps/client/src/widgets/collections/board/index.spec.tsx index e6635951b5..9e75cf0e26 100644 --- a/apps/client/src/widgets/collections/board/index.spec.tsx +++ b/apps/client/src/widgets/collections/board/index.spec.tsx @@ -211,6 +211,14 @@ function columnIcons(container: HTMLElement) { .map(el => [ ...el.classList ].filter(name => name.startsWith("bx")).join(" ")); } +/** + * A stand-in note context, with the two calls every mounted board makes of one: it publishes its + * columns into the context for the right pane to list, and clears them as it goes. + */ +function contextStub(context: object) { + return { setContextData: () => {}, clearContextData: () => {}, ...context }; +} + function columnTitles(container: HTMLElement) { return [ ...container.querySelectorAll(".board-column h3 .title") ].map(el => el.textContent); } @@ -795,7 +803,7 @@ describe("A board in a tab the reader is not looking at", () => { const otherTab = {}; let shown: object = tab; const host = new Component(); - Object.assign(host, { noteContext: { getMainContext: () => tab } }); + Object.assign(host, { noteContext: contextStub({ getMainContext: () => tab }) }); const previousTabManager = appContext.tabManager; appContext.tabManager = { getActiveMainContext: () => shown } as never; @@ -862,7 +870,7 @@ describe("A board in a tab the reader is not looking at", () => { // One tab, and the board is in a pane of it that does not hold the focus. const tab = {}; const host = new Component(); - Object.assign(host, { noteContext: { getMainContext: () => tab } }); + Object.assign(host, { noteContext: contextStub({ getMainContext: () => tab }) }); const previousTabManager = appContext.tabManager; appContext.tabManager = { getActiveMainContext: () => tab } as never; @@ -1471,6 +1479,28 @@ describe("Board column rename", () => { await act(async () => { await flush(); }); } + /** + * Two columns under one name would be merged by the rename, which a reader who has forgotten + * the other column does not expect. The name is refused, and the editor stays open on it. + */ + it("refuses a name another column already has and keeps the editor open", async () => { + const { container } = await setup(); + const put = vi.spyOn(server, "put").mockResolvedValue(undefined); + const message = vi.spyOn(toast, "showMessage").mockReturnValue(undefined); + + await renameColumnAt(container, 1, "Done"); + + expect(message).toHaveBeenCalledWith( + "board_view.column-name-taken:{\"column\":\"Done\"}", undefined, "bx bx-duplicate"); + expect(put).not.toHaveBeenCalled(); + + const columns = container.querySelectorAll(".board-column"); + expect(columns).toHaveLength(3); + // The column being renamed shows the editor in place of its title. + expect(columnTitles(container)).toEqual([ "To Do", "Done" ]); + expect(columns[1].querySelector("h3 input")?.value).toBe("Done"); + }); + /** * The cards, the stored columns and the definition are renamed together by the server, so that * no client reads the board while they disagree and writes the old name back. What the board @@ -4212,7 +4242,7 @@ describe("Board properties from the note menu", () => { }); const host = new Component(); - Object.assign(host, { noteContext: { ntxId, isActive: () => true } }); + Object.assign(host, { noteContext: contextStub({ ntxId, isActive: () => true }) }); const mountPoint = document.createElement("div"); container = mountPoint; @@ -4282,6 +4312,20 @@ describe("a column windowed for its size", () => { .toBeUndefined(); }); + /** + * The drag places a card at a column's foot by this count. It has to stand on the column + * itself: a collapsed column draws no card area to hang it off, and a windowed one draws fewer + * cards than it holds. + */ + it("says on each column how many cards it holds, drawn or not", async () => { + const board = await renderSized(200, 3); + const columns = board.querySelectorAll(".board-column"); + + expect(columns[0].dataset.count).toBe("200"); + expect(columns[0].querySelectorAll(".board-note").length).toBeLessThan(200); + expect(columns[1].dataset.count).toBe("3"); + }); + /** * The drag and the keyboard both name a card by the place it holds in its column. Counting * drawn elements would name a place among whatever is on screen, which moves as it scrolls. @@ -4332,3 +4376,153 @@ describe("a column windowed for its size", () => { return mountPoint; } }); + +describe("how wide the board draws its columns", () => { + let container: HTMLElement | undefined; + + afterEach(() => { + saved.length = 0; + if (container) { + render(null, container); + container.remove(); + container = undefined; + } + }); + + /** + * The width itself is CSS, which happy-dom does not resolve. What the board answers for is the + * class it carries, off which the stylesheet picks one of the three widths. + */ + it("carries the width its label names, and no class where it names none", async () => { + expect((await draw("wide")).className).toContain("board-wide-columns"); + expect((await draw("narrow")).className).toContain("board-narrow-columns"); + + // Without a class the width is whatever is inherited, which is the stylesheet's own + // default or a theme's if one sets it. + expect((await draw(undefined)).className).not.toMatch(/board-\w+-columns/); + expect((await draw("enormous")).className).not.toMatch(/board-\w+-columns/); + }); + + async function draw(width: string | undefined) { + if (container) { + render(null, container); + container.remove(); + } + + const note = buildNote({ + title: "Board", + "#collection": "", + "#viewType": "board", + ...(width ? { "#boardCardWidth": width } : {}), + children: [ { title: "First", "#status": "To Do" } ] + }); + + const mountPoint = document.createElement("div"); + container = mountPoint; + document.body.appendChild(mountPoint); + + await act(async () => { + render( + + + , + mountPoint + ); + }); + await act(async () => { await flush(); }); + + const board = mountPoint.querySelector(".board-view"); + if (!board) throw new Error("expected the board to be drawn"); + return board; + } +}); + +describe("what the board hands the right pane", () => { + let container: HTMLElement | undefined; + + afterEach(() => { + saved.length = 0; + if (container) { + render(null, container); + container.remove(); + container = undefined; + } + }); + + /** + * The pane lists the columns of whichever board is on show, so the board publishes them into + * the context it belongs to rather than the pane reading the board's own state. + */ + it("publishes its columns, scrolls to one on request, and takes them back", async () => { + const published: { key: string, value: unknown }[] = []; + const cleared: string[] = []; + const note = buildNote({ + title: "Board", + "#collection": "", + "#viewType": "board", + children: [ + { title: "First", "#status": "To Do" }, + { title: "Second", "#status": "Done" }, + { title: "Third", "#status": "Done" } + ] + }); + + const host = new Component(); + Object.assign(host, { noteContext: { + setContextData: (key: string, value: unknown) => published.push({ key, value }), + clearContextData: (key: string) => cleared.push(key) + } }); + + const mountPoint = document.createElement("div"); + container = mountPoint; + document.body.appendChild(mountPoint); + + await act(async () => { + render( + + + , + mountPoint + ); + }); + await act(async () => { await flush(); }); + + const outline = published.at(-1); + expect(outline?.key).toBe("boardColumns"); + const { columns, scrollToColumn } = outline?.value as { + columns: { value: string, title: string, count: number }[], + scrollToColumn: (column: string) => void + }; + expect(columns.map(({ title, count }) => `${title}:${count}`)) + .toEqual([ "To Do:1", "Done:2" ]); + + // What a press on one of the pane's entries does. Both are recorded rather than watched + // for: happy-dom scrolls nothing, and a dialog another spec left standing holds the focus. + const scrolled: (string | undefined)[] = []; + const focused: (string | undefined)[] = []; + for (const element of mountPoint.querySelectorAll(".board-column")) { + element.scrollIntoView = () => scrolled.push(element.dataset.column); + const heading = element.querySelector("h3"); + if (heading) { + heading.focus = () => focused.push(element.dataset.column); + } + } + + scrollToColumn("Done"); + expect(scrolled).toEqual([ "Done" ]); + // The heading takes the focus with it, so the board's own keys carry on from there. + expect(focused).toEqual([ "Done" ]); + + // A board that is no longer on show leaves nothing behind for the pane to list. + act(() => { render(null, mountPoint); }); + expect(cleared).toContain("boardColumns"); + }); +}); diff --git a/apps/client/src/widgets/collections/board/index.tsx b/apps/client/src/widgets/collections/board/index.tsx index f4019ed4e4..2941ad830b 100644 --- a/apps/client/src/widgets/collections/board/index.tsx +++ b/apps/client/src/widgets/collections/board/index.tsx @@ -31,8 +31,8 @@ import CollectionProperties from "../../note_bars/CollectionProperties"; import FormTextArea from "../../react/FormTextArea"; import FormTextBox from "../../react/FormTextBox"; import { - useContextualShortcutHints, useNoteContext, useNoteLabelBoolean, useNoteLabelWithDefault, - useNoteTypeOptions, useTrackedElement, useTriliumEvent + useContextualShortcutHints, useNoteContext, useNoteLabel, useNoteLabelBoolean, + useNoteLabelWithDefault, useNoteTypeOptions, useSetContextData, useTrackedElement, useTriliumEvent } from "../../react/hooks"; import Icon from "../../react/Icon"; import NoteAutocomplete from "../../react/NoteAutocomplete"; @@ -53,7 +53,10 @@ import { forgetWindowHeights } from "./windowing"; import { BoardDropStateContext, DropStateStore } from "./drop_state"; import BoardApi from "./api"; import { adoptLegacyColumns, readColumns } from "./column_storage"; -import { DEFAULT_COLUMN_ICON, DEFAULT_GROUP_BY, getStatusDefinition, INBOX_COLUMN } from "./columns"; +import { + COLUMN_WIDTH_LABEL, columnWidthClass, DEFAULT_COLUMN_ICON, DEFAULT_GROUP_BY, + getStatusDefinition, INBOX_COLUMN +} from "./columns"; import Column, { EXPAND_MS, placeCard, settleCards } from "./column"; import { currentCardTemplate, DEFAULT_CARD_TEMPLATES } from "./card_templates"; import ColumnLimitDialog from "./column_limit"; @@ -351,6 +354,9 @@ export default function BoardView({ let viewConfig = adoptedConfig ?? storedConfig; const [ includeArchived ] = useNoteLabelBoolean(parentNote, "includeArchived"); const [ inboxEnabled ] = useNoteLabelBoolean(parentNote, "enableInboxColumn"); + // Read undefaulted: a board naming no width wears no class, so `--board-column-width` keeps + // whatever it inherits. + const [ storedColumnWidth ] = useNoteLabel(parentNote, COLUMN_WIDTH_LABEL); /** Every card the board holds, which an active filter narrows before the cards are drawn. */ const [ allByColumn, setAllByColumn ] = useState(); const [ columns, setColumns ] = useState(); @@ -626,6 +632,24 @@ export default function BoardView({ /** Until when a column move can still be settling, which is when `useFlip` slides columns. */ const columnMovedUntil = useRef(0); + // What the right pane lists the board as, and what a press on one of its entries does. + // + // Held still between renders: the pane redraws its list whenever this changes identity, and a + // board renders on every step of a drag. `storedColumns` is among what it is held against + // because the api reads the config in place, so nothing else here changes when it does. + const outline = useMemo(() => ({ + columns: api.getColumnOutline(shownColumns), + scrollToColumn: (column: string) => { + const element = columnElement(containerRef.current, column); + element?.scrollIntoView({ inline: "start", block: "nearest", behavior: "smooth" }); + // The heading takes the focus, so the board's own keys carry on from the column the + // reader picked. Scrolled first, and without moving anything itself: focus landing on + // its own would jump the board to the column the scroll is already easing towards. + element?.querySelector("h3")?.focus({ preventScroll: true }); + } + }), [ api, shownColumns, byColumn, storedColumns, isInRelationMode ]); + useSetContextData(noteContext, "boardColumns", outline); + // Neither the creation dates the tie-break needs nor the targets of a sorted relation come // with the board. Both are fetched here, and `sortRevision` redraws it once they land. useEffect(() => { @@ -965,13 +989,8 @@ export default function BoardView({ if (!isMobile()) return; requestAnimationFrame(() => { - const columns = containerRef.current?.querySelectorAll(".board-column"); - for (const element of columns ?? []) { - if (element.dataset.column === column) { - element.scrollIntoView({ inline: "center", block: "nearest" }); - return; - } - } + columnElement(containerRef.current, column) + ?.scrollIntoView({ inline: "center", block: "nearest" }); }); }, []); @@ -1198,7 +1217,7 @@ export default function BoardView({ : undefined; return ( -
0 })}> @@ -1410,6 +1429,15 @@ function closeGaps(container: HTMLElement | null) { } } +/** The element a column is drawn in, for the two things that scroll the board to one. */ +function columnElement(container: HTMLElement | null, column: string) { + for (const element of container?.querySelectorAll(".board-column") ?? []) { + if (element.dataset.column === column) { + return element; + } + } +} + export function findRefreshReason(loadResults: LoadResults, statusAttribute: string, noteIds: string[], parentNoteId: string): string | null { // A card moved between columns. if (loadResults.getAttributeRows().some(attr => attr.name === statusAttribute && noteIds.includes(attr.noteId ?? ""))) { @@ -1536,7 +1564,11 @@ export function TitleEditor({ }: { currentValue?: string; placeholder?: string; - save: (newValue: string, atStart?: boolean) => void | Promise; + /** + * Writes what was typed. Returns `false` to refuse it, which keeps the editor open on what it + * holds so the reader can correct it. + */ + save: (newValue: string, atStart?: boolean) => false | void | Promise; dismiss: () => void; isNewItem?: boolean; mode?: "normal" | "multiline" | "relation"; @@ -1678,8 +1710,9 @@ export function TitleEditor({ // editor opened by a press on the thing it edits, rather than from something focused, // has nowhere to send it, so Enter says here what that blur would have said. const typed = inputRef.current?.value ?? ""; - if (e.key === "Enter" && typed.trim() && (typed !== currentValue || isNewItem)) { - commit(typed); + if (e.key === "Enter" && typed.trim() && (typed !== currentValue || isNewItem) + && !commit(typed)) { + return; } dismiss(); @@ -1701,7 +1734,10 @@ export function TitleEditor({ return; } - commit(value, atStart); + if (!commit(value, atStart)) { + input?.focus(); + return; + } if (handsOver) { hasHandedOver.current = true; @@ -1782,21 +1818,36 @@ export function TitleEditor({ } if (!shouldDismiss.current && newValue.trim() && (newValue !== currentValue || isNewItem)) { - commit(newValue); + if (!commit(newValue)) { + // The field stays open, so the focus this blur took off it has to come back. + inputRef.current?.focus(); + return; + } + dismissOnNextRefreshRef.current = true; } else { dismiss(); } }; - // The editor is closing either way, and what a save writes has already been put back by - // whatever could not write it; all that is left is to say so rather than to reject unhandled, - // which is what a save reaching nobody used to do. + /** + * Saves what was typed and reports whether `save` accepted it. + * + * A refusal is reported by `save` itself, which is what knows why it refused. A save that + * fails later is reported here instead of rejecting unhandled: the editor has closed by then, + * and whatever could not be written has already been put back. + */ function commit(newValue: string, atStart?: boolean) { - Promise.resolve(save(newValue, atStart)).catch((e) => { + const outcome = save(newValue, atStart); + if (outcome === false) { + return false; + } + + Promise.resolve(outcome).catch((e) => { console.error("Failed to save what the board editor was given:", e); toast.showError(t("board_view.save-error")); }); + return true; } if (mode !== "relation") { @@ -1908,7 +1959,10 @@ export function TitleEditor({ }} onBlur={() => dismiss()} noteIdChanged={(newValue) => { - save(newValue); + if (newValue && !commit(newValue)) { + return; + } + dismiss(); }} /> diff --git a/apps/client/src/widgets/collections/board/properties.spec.tsx b/apps/client/src/widgets/collections/board/properties.spec.tsx index 6c14985dbd..4c0860469c 100644 --- a/apps/client/src/widgets/collections/board/properties.spec.tsx +++ b/apps/client/src/widgets/collections/board/properties.spec.tsx @@ -62,6 +62,8 @@ describe("Board properties", () => { let labels: Record; /** What the board was asked to do with the order it holds. */ let sorting: string[]; + /** The widths the board was asked to draw its columns at. */ + let widths: string[]; /** Draws the dialog again, for a test that changed what the board says. */ let draw: () => void; @@ -70,6 +72,7 @@ describe("Board properties", () => { storedAttributes = []; toggled = []; sorting = []; + widths = []; labels = { includeArchived: "true" }; container = document.createElement("div"); document.body.appendChild(container); @@ -102,7 +105,8 @@ describe("Board properties", () => { setDefaultSortDirection: async (isDescending: boolean) => { sorting.push(`descending:${isDescending}`); }, - resetColumnSortsToDefault: async () => { sorting.push("reset"); } + resetColumnSortsToDefault: async () => { sorting.push("reset"); }, + setColumnWidth: async (width: string) => { widths.push(width); } } as unknown as BoardApi; draw = () => act(() => { @@ -154,7 +158,7 @@ describe("Board properties", () => { const rows = general() ?.querySelectorAll(".tn-card-section:not(.tn-card-section-nested)") ?? []; - expect(rows.length).toBe(3); + expect(rows.length).toBe(4); expect(toggleAt(0)?.classList.contains("on")).toBe(false); expect(toggleAt(1)?.classList.contains("on")).toBe(true); }); @@ -217,6 +221,44 @@ describe("Board properties", () => { } }); + describe("how wide it draws its columns", () => { + it("offers the three widths the board knows", () => { + // happy-dom does not follow which option Preact marks as selected, so what the board + // holds is covered by `parseColumnWidth` and the api instead. + expect([ ...(picker()?.options ?? []) ].map((option) => option.textContent)).toEqual([ + "board_view.column-width-narrow", + "board_view.column-width-medium", + "board_view.column-width-wide" + ]); + }); + + it("asks the board for the width that was picked", () => { + const select = picker(); + if (!select) throw new Error("expected a width picker on the board properties"); + + act(() => { + select.value = "medium"; + select.dispatchEvent(new Event("change", { bubbles: true })); + }); + + expect(widths).toEqual([ "medium" ]); + }); + + /** A phone lays the columns out at a width of its own, so there is nothing to pick. */ + it("is left out on mobile", () => { + window.glob.device = "mobile"; + act(() => { render(null, container); }); + draw(); + + expect(picker()).toBeFalsy(); + window.glob.device = "desktop"; + }); + + function picker() { + return general()?.querySelector("select"); + } + }); + it("holds the promoted attributes, in the board's own words and from its own config", () => { const card = dialog()?.querySelector(".attributes-stub"); diff --git a/apps/client/src/widgets/collections/board/properties.tsx b/apps/client/src/widgets/collections/board/properties.tsx index a971fbfc95..6b28ad74aa 100644 --- a/apps/client/src/widgets/collections/board/properties.tsx +++ b/apps/client/src/widgets/collections/board/properties.tsx @@ -5,9 +5,11 @@ import { useCallback } from "preact/hooks"; import type FNote from "../../../entities/fnote"; import dialog from "../../../services/dialog"; import { t } from "../../../services/i18n"; +import { isMobile } from "../../../services/utils"; import { Card, CardSection, OptionCardSection } from "../../react/Card"; +import FormSelect from "../../react/FormSelect"; import FormToggle from "../../react/FormToggle"; -import { useNoteLabelBoolean } from "../../react/hooks"; +import { useNoteLabelBoolean, useNoteLabelWithDefault } from "../../react/hooks"; import Modal from "../../react/Modal"; import PromotedAttributesCard from "../../react/PromotedAttributesCard"; import TemplateSelectionCard from "../../react/TemplateSelectionCard"; @@ -15,6 +17,7 @@ import type { PromotedAttribute } from "../promoted_attributes"; import SortDropdown from "../SortDropdown"; import { parseSortKey } from "../sorting"; import BoardApi from "./api"; +import { COLUMN_WIDTH_LABEL, DEFAULT_COLUMN_WIDTH, parseColumnWidth } from "./columns"; import { useBoardSort } from "./sort"; /** The board's settings, other than its columns and cards. */ @@ -66,6 +69,8 @@ export default function BoardProperties({ api, note, shown, onClose }: { function General({ api, note }: { api: BoardApi, note: FNote }) { const [ inboxShown ] = useNoteLabelBoolean(note, "enableInboxColumn"); const [ archivedShown ] = useNoteLabelBoolean(note, "includeArchived"); + const [ columnWidth ] = + useNoteLabelWithDefault(note, COLUMN_WIDTH_LABEL, DEFAULT_COLUMN_WIDTH); const defaultSort = useBoardSort(note); // Every column at once, and a column's own order is not kept anywhere else: the reader is asked // before it goes. @@ -98,6 +103,27 @@ function General({ api, note }: { api: BoardApi, note: FNote }) { /> + {/* `body.mobile` in index.css fixes the column width against the viewport, which is + what the three widths here would otherwise set. */} + {!isMobile() && ( + + api.setColumnWidth(parseColumnWidth(width))} + /> + + )} + ({ + board: null as NoteContextDataMap["boardColumns"] | null +})); +vi.mock("../../services/i18n", () => ({ t: (key: string) => key })); +vi.mock("../react/hooks", async (importOriginal) => ({ + ...(await importOriginal()), + useGetContextData: () => shown.board +})); + +let container: HTMLDivElement; + +beforeEach(() => { + // The widget stands in a RightPanelWidget, which reads which panels the user collapsed. + options.set("rightPaneCollapsedItems", JSON.stringify([])); + shown.board = { + columns: [ + { value: "", title: "Inbox", icon: "bx bxs-inbox", count: 2 }, + { value: "To Do", title: "To Do", icon: "bx bx-circle", count: 7 }, + { value: "Done", title: "Done", icon: "bx bx-check", count: 0 } + ], + scrollToColumn: vi.fn() + }; + + container = document.createElement("div"); + document.body.append(container); +}); + +afterEach(() => { + act(() => render(null, container)); + container.remove(); + shown.board = null; +}); + +describe("BoardColumns", () => { + it("lists the columns in the board's own order, each with its icon and how many it holds", () => { + renderPanel(); + + const entries = [ ...container.querySelectorAll(".board-column-entry") ]; + expect(entries.map((entry) => entry.querySelector(".board-column-name")?.textContent)) + .toEqual([ "Inbox", "To Do", "Done" ]); + expect(entries.map((entry) => entry.querySelector(".badge")?.textContent)) + .toEqual([ "2", "7", "0" ]); + expect(entries[1].querySelector("span.bx-circle")).toBeTruthy(); + }); + + it("asks the board to scroll to the column that was pressed, by mouse or by keyboard", () => { + renderPanel(); + const entries = [ ...container.querySelectorAll(".board-column-entry") ]; + + entries[2].click(); + expect(shown.board?.scrollToColumn).toHaveBeenCalledWith("Done"); + + act(() => { + entries[0].dispatchEvent( + new KeyboardEvent("keydown", { key: "Enter", bubbles: true })); + }); + // The inbox is named by the empty value, which is what it is identified by throughout. + expect(shown.board?.scrollToColumn).toHaveBeenLastCalledWith(""); + }); + + it("says so for a board with no columns at all", () => { + shown.board = { columns: [], scrollToColumn: vi.fn() }; + renderPanel(); + + expect(container.querySelector(".board-column-entry")).toBeNull(); + expect(container.querySelector(".no-columns")?.textContent) + .toBe("board_view.columns-empty"); + }); +}); + +function renderPanel() { + act(() => render(, container)); +} diff --git a/apps/client/src/widgets/sidebar/BoardColumns.tsx b/apps/client/src/widgets/sidebar/BoardColumns.tsx new file mode 100644 index 0000000000..2b5648638c --- /dev/null +++ b/apps/client/src/widgets/sidebar/BoardColumns.tsx @@ -0,0 +1,51 @@ +import "./BoardColumns.css"; + +import { t } from "../../services/i18n"; +import SimpleBadge from "../react/Badge"; +import { useGetContextData } from "../react/hooks"; +import Icon from "../react/Icon"; +import RightPanelWidget from "./RightPanelWidget"; + +/** + * The board's columns, in the order it draws them, so that one of them can be reached on a board + * too wide to see at once. The board publishes the list and the scrolling; this widget draws it. + */ +export default function BoardColumns() { + const data = useGetContextData("boardColumns"); + const columns = data?.columns ?? []; + + return ( + +
+ {columns.length > 0 ? ( +
    + {columns.map(column => ( +
  1. + {/* The row is the scroll target, reachable by keyboard as the + other lists of the pane are. */} + data?.scrollToColumn(column.value)} + onKeyDown={e => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + data?.scrollToColumn(column.value); + } + }} + > + + {column.title} + + +
    2. + ))} +
+ ) : ( +
{t("board_view.columns-empty")}
+ )} +
+ + ); +} diff --git a/apps/client/src/widgets/sidebar/RightPanelContainer.tsx b/apps/client/src/widgets/sidebar/RightPanelContainer.tsx index f4c1b16ce1..3bddb9266a 100644 --- a/apps/client/src/widgets/sidebar/RightPanelContainer.tsx +++ b/apps/client/src/widgets/sidebar/RightPanelContainer.tsx @@ -20,6 +20,7 @@ import { PaneMode, usePaneMode, usePeekDismiss } from "../react/peek_pane"; import LegacyRightPanelWidget from "../right_panel_widget"; import AttributeList from "./AttributeList"; import Backlinks from "./Backlinks"; +import BoardColumns from "./BoardColumns"; import ChatHighlightsList from "./ChatHighlightsList"; import HighlightsList from "./HighlightsList"; import NoteMap from "./NoteMap"; @@ -203,6 +204,8 @@ function useItems(rightPaneVisible: boolean, widgetsByParent: WidgetsByParent): // Published by the LLM chat; drives the chat highlights widget's visibility (only shown once // the chat has at least one highlight). const chatHighlights = useGetContextData("chatHighlights"); + // Published by a board, which is what says the note is being shown as one. + const boardColumns = useGetContextData("boardColumns"); // Subscribe to the AI toggle so the LLM chat is added/removed reactively without a page reload. const [ aiEnabled ] = useTriliumOptionBool("aiEnabled"); const isPdf = noteType === "file" && noteMime === "application/pdf"; @@ -268,6 +271,13 @@ function useItems(rightPaneVisible: boolean, widgetsByParent: WidgetsByParent): enabled: (noteType === "text" || !!note?.isMarkdown()) && highlightsList.length > 0, tab: "outline" }, + { + el: , + // Gated on being a board rather than on having columns, so that a board with none of + // them shows the card's own empty state. + enabled: boardColumns !== undefined, + tab: "outline" + }, { el: , enabled: noteType === "llmChat" && (chatHighlights?.highlights.length ?? 0) > 0, diff --git a/apps/icon-pack-builder/package.json b/apps/icon-pack-builder/package.json index 0a6aaffff8..08dcdc2c66 100644 --- a/apps/icon-pack-builder/package.json +++ b/apps/icon-pack-builder/package.json @@ -7,8 +7,12 @@ "start": "tsx ." }, "keywords": [], + "dependencies": { + "@triliumnext/commons": "workspace:*" + }, "devDependencies": { "@mdi/font": "7.4.47", - "@phosphor-icons/web": "2.1.2" + "@phosphor-icons/web": "2.1.2", + "opentype.js": "2.0.0" } } diff --git a/apps/icon-pack-builder/src/provider.ts b/apps/icon-pack-builder/src/provider.ts index 533d592c53..1a73a11473 100644 --- a/apps/icon-pack-builder/src/provider.ts +++ b/apps/icon-pack-builder/src/provider.ts @@ -1,4 +1,4 @@ -import type { IconPackManifest } from "@triliumnext/server/src/services/icon_packs"; +import type { IconPackManifest } from "@triliumnext/core/src/services/icon_packs"; export interface IconPackData { name: string; diff --git a/apps/icon-pack-builder/src/providers/boxicons3.ts b/apps/icon-pack-builder/src/providers/boxicons3.ts index d2f9187d0e..c74a0a4508 100644 --- a/apps/icon-pack-builder/src/providers/boxicons3.ts +++ b/apps/icon-pack-builder/src/providers/boxicons3.ts @@ -2,6 +2,7 @@ import { readFileSync } from "fs"; import { join } from "path"; import { IconPackData } from "../provider"; +import { readIconFontMetrics } from "../utils"; export default function buildIcons(pack: "basic" | "brands"): IconPackData { const inputDir = join(__dirname, "../../boxicons-free/fonts"); @@ -36,7 +37,8 @@ export default function buildIcons(pack: "basic" | "brands"): IconPackData { content: readFileSync(join(inputDir, pack, `${fileName}.woff2`)) }, manifest: { - icons + icons, + metrics: readIconFontMetrics(join(inputDir, pack, `${fileName}.ttf`)) }, meta: { version: "3.0.0", diff --git a/apps/icon-pack-builder/src/providers/mdi.ts b/apps/icon-pack-builder/src/providers/mdi.ts index 381818d903..7e7567076f 100644 --- a/apps/icon-pack-builder/src/providers/mdi.ts +++ b/apps/icon-pack-builder/src/providers/mdi.ts @@ -2,7 +2,7 @@ import { readFileSync } from "fs"; import { join } from "path"; import type { IconPackData } from "../provider"; -import { extractClassNamesFromCss, getModulePath } from "../utils"; +import { extractClassNamesFromCss, getModulePath, readIconFontMetrics } from "../utils"; export default function buildIcons(): IconPackData { const baseDir = getModulePath("@mdi/font"); @@ -17,6 +17,7 @@ export default function buildIcons(): IconPackData { icon: "mdi mdi-material-design", manifest: { icons: extractClassNamesFromCss(cssFileContent, "mdi"), + metrics: readIconFontMetrics(join(baseDir, "fonts", "materialdesignicons-webfont.ttf")) }, fontFile: { name: "materialdesignicons-webfont.woff2", diff --git a/apps/icon-pack-builder/src/providers/phosphor.ts b/apps/icon-pack-builder/src/providers/phosphor.ts index 5c6a3e225a..923ce6c348 100644 --- a/apps/icon-pack-builder/src/providers/phosphor.ts +++ b/apps/icon-pack-builder/src/providers/phosphor.ts @@ -2,7 +2,7 @@ import { readdirSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { IconPackData } from "../provider"; -import { getModulePath } from "../utils"; +import { getModulePath, readIconFontMetrics } from "../utils"; export default function buildIcons(packName: "regular" | "fill"): IconPackData { const moduleDir = getModulePath("@phosphor-icons/web"); @@ -30,6 +30,10 @@ export default function buildIcons(packName: "regular" | "fill"): IconPackData { } const fontFile = readdirSync(baseDir).find(f => f.endsWith(".woff2")); + if (!fontFile) { + throw new Error(`No WOFF2 font to build the pack from in ${baseDir}.`); + } + const prefix = packName === "regular" ? "ph" : `ph-${packName}`; return { @@ -37,12 +41,13 @@ export default function buildIcons(packName: "regular" | "fill"): IconPackData { prefix, icon: `${prefix} ph-phosphor-logo`, manifest: { - icons + icons, + metrics: readIconFontMetrics(join(baseDir, fontFile.replace(/\.woff2$/, ".ttf"))) }, fontFile: { - name: fontFile!, + name: fontFile, mime: "font/woff2", - content: readFileSync(join(baseDir, fontFile!)) + content: readFileSync(join(baseDir, fontFile)) }, meta: { version: packageJson.version, diff --git a/apps/icon-pack-builder/src/utils.ts b/apps/icon-pack-builder/src/utils.ts index b88dc0e6bd..bbd5ce9410 100644 --- a/apps/icon-pack-builder/src/utils.ts +++ b/apps/icon-pack-builder/src/utils.ts @@ -1,6 +1,10 @@ +import { readFileSync } from "fs"; import { join } from "path"; -import { IconPackManifest } from "../../server/src/services/icon_packs"; +import { measureIconFont } from "@triliumnext/commons"; +import opentype from "opentype.js"; + +import { IconPackManifest } from "@triliumnext/core/src/services/icon_packs"; export function extractClassNamesFromCss(css: string, prefix: string): IconPackManifest["icons"] { const regex = /\.([a-zA-Z0-9-]+)::before\s*\{\s*content:\s*"\\([A-Fa-f0-9]+)"\s*\}/g; @@ -24,3 +28,36 @@ export function extractClassNamesFromCss(css: string, prefix: string): IconPackM export function getModulePath(moduleName: string): string { return join(__dirname, "../../../node_modules", moduleName); } + +/** + * The metrics of an icon font, read off its glyphs, for the pack's manifest to carry. + * + * Packs ship as WOFF2, which cannot be taken apart without a Brotli decoder, so this reads the + * TrueType build every provider has beside it. The two hold the same outlines. + * + * @param fontPath the `.ttf` to measure. + * @returns the metrics, or `undefined` where the font cannot be read. + */ +export function readIconFontMetrics(fontPath: string): IconPackManifest["metrics"] { + let font; + try { + const bytes = readFileSync(fontPath); + font = opentype.parse(bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.length)); + } catch (e) { + console.warn(`Could not read font metrics from ${fontPath}: ${e}`); + return undefined; + } + + const inkCentres: number[] = []; + for (const glyph of Object.values(font.glyphs.glyphs)) { + if (!glyph.unicode) continue; + + const box = glyph.getBoundingBox(); + // A blank glyph reports an inverted box, and says nothing about where the pack draws. + if (box.y2 > box.y1) { + inkCentres.push((box.y1 + box.y2) / 2); + } + } + + return measureIconFont(font.unitsPerEm, inkCentres) ?? undefined; +} diff --git a/packages/commons/src/index.ts b/packages/commons/src/index.ts index cf140e0042..6f561454f3 100644 --- a/packages/commons/src/index.ts +++ b/packages/commons/src/index.ts @@ -8,6 +8,7 @@ export * from "./lib/test-utils.js"; export * from "./lib/attachment_roles.js"; export * from "./lib/custom_fonts.js"; export * from "./lib/font_mimes.js"; +export * from "./lib/icon_font_metrics.js"; export * from "./lib/image_mimes.js"; export * from "./lib/mime_type.js"; export * from "./lib/office.js"; diff --git a/packages/commons/src/lib/builtin_attributes.ts b/packages/commons/src/lib/builtin_attributes.ts index 270b7f3c09..57595e6e4c 100644 --- a/packages/commons/src/lib/builtin_attributes.ts +++ b/packages/commons/src/lib/builtin_attributes.ts @@ -271,6 +271,9 @@ const BUILTIN_ATTRIBUTES = [ { type: "label", name: "enableInboxColumn", valueType: "boolean", hasUserValue: true }, // Carried by a card that stands in for another note: opening it navigates there instead. { type: "relation", name: "boardCardRedirectTo" }, + // How wide the board draws its columns. Absent for the narrow default. + { type: "label", name: "boardCardWidth", valueType: "select", hasUserValue: true, + selectOptions: [ "narrow", "medium", "wide" ] }, // The order a board offers for its columns, which its properties apply to every column at once. { type: "label", name: "sortColumns", valueType: "text", hasUserValue: true }, { type: "label", name: "sortColumnsDescending", valueType: "boolean", hasUserValue: true }, diff --git a/packages/commons/src/lib/icon_font_metrics.spec.ts b/packages/commons/src/lib/icon_font_metrics.spec.ts new file mode 100644 index 0000000000..a1d52070a3 --- /dev/null +++ b/packages/commons/src/lib/icon_font_metrics.spec.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; + +import { iconFontFaceOverrides, measureIconFont } from "./icon_font_metrics.js"; + +/** Ink centres for a pack drawn `offset` units above the baseline, with a little scatter. */ +function pack(offset: number, count = 20) { + return Array.from({ length: count }, (_, index) => offset + (index % 5) - 2); +} + +describe("measureIconFont", () => { + it("splits the em around the middle of the ink", () => { + // Boxicons 2: 1024 units to the em, drawn 448 of them above the baseline. + expect(measureIconFont(1024, pack(448))).toEqual({ ascent: 0.9375, descent: 0.0625 }); + + // Boxicons 3: the em sits on the baseline, so the box is all ascent. + expect(measureIconFont(300, pack(150))).toEqual({ ascent: 1, descent: 0 }); + + // Material Design Icons: 512 units to the em, 192 above the baseline. + expect(measureIconFont(512, pack(192))).toEqual({ ascent: 0.875, descent: 0.125 }); + + // A pack of an odd number of glyphs, where the middle one is the median outright. + expect(measureIconFont(1024, pack(448, 21))).toEqual({ ascent: 0.9375, descent: 0.0625 }); + }); + + it("takes the middle of the pack, not the icons drawn off centre on purpose", () => { + const centres = [ ...pack(448), 0, 12, 1000, 1010 ]; + + expect(measureIconFont(1024, centres)).toEqual({ ascent: 0.9375, descent: 0.0625 }); + }); + + it("declines a font it cannot read a box out of", () => { + expect(measureIconFont(0, pack(448))).toBeNull(); + expect(measureIconFont(-1024, pack(448))).toBeNull(); + expect(measureIconFont(1024, [])).toBeNull(); + expect(measureIconFont(1024, pack(448, 7))).toBeNull(); + + // Ink so far from the em that the overrides would come out negative. + expect(measureIconFont(1024, pack(-900))).toBeNull(); + expect(measureIconFont(1024, pack(2600))).toBeNull(); + }); +}); + +describe("iconFontFaceOverrides", () => { + it("writes the descriptors a font face carries", () => { + expect(iconFontFaceOverrides({ ascent: 0.9375, descent: 0.0625 })).toEqual([ + "ascent-override: 93.75%;", + "descent-override: 6.25%;", + "line-gap-override: 0%;" + ]); + }); + + it("declares nothing for a pack that gave no usable metrics", () => { + expect(iconFontFaceOverrides(undefined)).toEqual([]); + expect(iconFontFaceOverrides({ ascent: 0.9, descent: NaN })).toEqual([]); + expect(iconFontFaceOverrides({ ascent: -0.1, descent: 0.1 })).toEqual([]); + expect(iconFontFaceOverrides({ ascent: 4, descent: 0.1 })).toEqual([]); + }); + + it("drops a manifest value that is not a number, which a pack's author writes by hand", () => { + const evil = { ascent: "0%; } body { display: none } @font-face { x: 1", descent: 0.1 }; + + expect(iconFontFaceOverrides(evil as never)).toEqual([]); + }); +}); diff --git a/packages/commons/src/lib/icon_font_metrics.ts b/packages/commons/src/lib/icon_font_metrics.ts new file mode 100644 index 0000000000..41e2a2e558 --- /dev/null +++ b/packages/commons/src/lib/icon_font_metrics.ts @@ -0,0 +1,96 @@ +/** + * Where an icon pack draws its glyphs, as fractions of the em. + * + * A browser takes the box it centres a glyph in from the font's platform metrics, which carry + * padding the pack never drew in: Boxicons 3 declares 300 units per em and a Windows ascent of 327, + * so each of its icons sits about 4.5% of the em below the middle of its container. Declaring the + * pack's own box through `ascent-override` and `descent-override` removes that, and pins the result + * across platforms that read different metrics out of the same file. + */ +export interface IconFontMetrics { + /** The `ascent-override` value. */ + ascent: number; + /** The `descent-override` value. */ + descent: number; +} + +/** Below this many inked glyphs the middle of the set says too little to override anything by. */ +const MIN_MEASURED_GLYPHS = 8; + +/** The largest override that can be meant seriously, as a fraction of the em. */ +const MAX_OVERRIDE = 2; + +/** + * The metrics of a pack, worked out from where it draws. + * + * The middle of the ink is taken over the whole pack rather than per glyph: individual icons are + * drawn off centre on purpose, and the median says where the box they share sits without following + * any of them. + * + * @param unitsPerEm the font's own unit scale. + * @param inkCentres the vertical middle of each glyph's ink, in font units above the baseline. + * @returns the metrics, or `null` where the font says too little to correct. + */ +export function measureIconFont(unitsPerEm: number, inkCentres: number[]): IconFontMetrics | null { + if (!(unitsPerEm > 0) || inkCentres.length < MIN_MEASURED_GLYPHS) { + return null; + } + + const middle = median(inkCentres) / unitsPerEm; + const ascent = 0.5 + middle; + const descent = 0.5 - middle; + if (!isOverride(ascent) || !isOverride(descent)) { + return null; + } + + return { ascent: roundFraction(ascent), descent: roundFraction(descent) }; +} + +/** + * The `@font-face` descriptors a pack's metrics stand for, one per line, or nothing where it + * declares none. + * + * A pack's manifest is written by whoever made it, so a value that is not a usable number is + * dropped rather than passed on to the stylesheet. + */ +export function iconFontFaceOverrides(metrics: IconFontMetrics | undefined): string[] { + const ascent = overridePercentage(metrics?.ascent); + const descent = overridePercentage(metrics?.descent); + if (ascent === null || descent === null) { + return []; + } + + return [ + `ascent-override: ${ascent}%;`, + `descent-override: ${descent}%;`, + "line-gap-override: 0%;" + ]; +} + +/** Whether a computed override is a number a `@font-face` can carry. */ +function isOverride(value: number) { + return Number.isFinite(value) && value >= 0 && value <= MAX_OVERRIDE; +} + +/** One override as a percentage, or `null` where the manifest's value cannot be used. */ +function overridePercentage(value: number | undefined) { + if (typeof value !== "number" || !isOverride(value)) { + return null; + } + + return Number((value * 100).toFixed(2)); +} + +/** Six decimals of the em, which is finer than any screen resolves. */ +function roundFraction(value: number) { + return Number(value.toFixed(6)); +} + +function median(values: number[]) { + const sorted = [ ...values ].sort((first, second) => first - second); + const middle = Math.floor(sorted.length / 2); + + return sorted.length % 2 === 0 + ? (sorted[middle - 1] + sorted[middle]) / 2 + : sorted[middle]; +} diff --git a/packages/trilium-core/src/services/icon_pack_boxicons-v2.json b/packages/trilium-core/src/services/icon_pack_boxicons-v2.json index 1ad1fb7926..b85712057e 100644 --- a/packages/trilium-core/src/services/icon_pack_boxicons-v2.json +++ b/packages/trilium-core/src/services/icon_pack_boxicons-v2.json @@ -1,4 +1,8 @@ { + "metrics": { + "ascent": 0.9375, + "descent": 0.0625 + }, "icons": { "bx-child": { "glyph": "", diff --git a/packages/trilium-core/src/services/icon_packs.spec.ts b/packages/trilium-core/src/services/icon_packs.spec.ts index f8c06820b2..a947a31f2f 100644 --- a/packages/trilium-core/src/services/icon_packs.spec.ts +++ b/packages/trilium-core/src/services/icon_packs.spec.ts @@ -164,6 +164,39 @@ describe("CSS generation", () => { expect(css).toContain(`.bx.bx-ball::before { content: "\ue9c2"; }`); expect(css).toContain(`.bx.bxs-party::before { content: "\uec92"; }`); }); + + it("declares the box a pack was measured in, and nothing where it was not", () => { + const measured = cssForManifest({ ...manifest, metrics: { ascent: 0.875, descent: 0.125 } }); + expect(measured).toContain("ascent-override: 87.5%;"); + expect(measured).toContain("descent-override: 12.5%;"); + expect(measured).toContain("line-gap-override: 0%;"); + + const unmeasured = cssForManifest(manifest); + expect(unmeasured).toContain("@font-face"); + expect(unmeasured).not.toContain("ascent-override"); + expect(unmeasured).not.toContain("descent-override"); + }); + + it("drops metrics a pack's author wrote by hand and got wrong", () => { + const evil = "0%; } body { display: none } @font-face { a: 1"; + + const css = cssForManifest({ ...manifest, metrics: { ascent: evil, descent: 0.1 } as never }); + expect(css).not.toContain("ascent-override"); + expect(css).not.toContain("display: none"); + }); + + function cssForManifest(iconPackManifest: IconPackManifest) { + const processed = processIconPack(buildNote({ + type: "text", + title: "Boxicons v2", + content: JSON.stringify(iconPackManifest), + attachments: [ defaultAttachment ], + "#iconPack": "bx" + })); + expect(processed).toBeTruthy(); + + return (processed && generateCss(processed, "/api/attachments/x/download")) ?? ""; + } }); describe("Generating CSS for untrusted manifests", () => { diff --git a/packages/trilium-core/src/services/icon_packs.ts b/packages/trilium-core/src/services/icon_packs.ts index 48487e11d3..421c451ef9 100644 --- a/packages/trilium-core/src/services/icon_packs.ts +++ b/packages/trilium-core/src/services/icon_packs.ts @@ -1,4 +1,4 @@ -import { IconRegistry } from "@triliumnext/commons"; +import { iconFontFaceOverrides, type IconFontMetrics, IconRegistry } from "@triliumnext/commons"; import type BAttachment from "../becca/entities/battachment"; import type BNote from "../becca/entities/bnote"; @@ -38,6 +38,12 @@ export interface IconPackManifest { glyph: string, terms: string[]; }>; + /** + * Where the pack draws its glyphs, so that a browser centres them on the box they were drawn in + * rather than on the one the font's platform metrics describe. Packs built before this was + * measured carry none, and are left to the browser. + */ + metrics?: IconFontMetrics; } export interface ProcessedIconPack { @@ -174,12 +180,17 @@ export function generateCss({ manifest, fontMime, builtin, fontAttachmentId, pre } const fontFamily = builtin ? fontAttachmentId : `trilium-icon-pack-${prefix}`; + const fontFace = [ + `font-family: '${fontFamily}';`, + "font-weight: normal;", + "font-style: normal;", + `src: url('${fontUrl}') format('${MIME_TO_CSS_FORMAT_MAPPINGS[fontMime]}');`, + ...iconFontFaceOverrides(manifest.metrics) + ].join("\n "); + return `\ @font-face { - font-family: '${fontFamily}'; - font-weight: normal; - font-style: normal; - src: url('${fontUrl}') format('${MIME_TO_CSS_FORMAT_MAPPINGS[fontMime]}'); + ${fontFace} } .${prefix} { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9e73950df0..6b2fb6ad6a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -558,6 +558,10 @@ importers: version: 44.2.0(supports-color@10.2.2) apps/icon-pack-builder: + dependencies: + '@triliumnext/commons': + specifier: workspace:* + version: link:../../packages/commons devDependencies: '@mdi/font': specifier: 7.4.47 @@ -565,6 +569,9 @@ importers: '@phosphor-icons/web': specifier: 2.1.2 version: 2.1.2 + opentype.js: + specifier: 2.0.0 + version: 2.0.0 apps/mobile: dependencies: