From a7c04f320d87ebf971cbc051b7fdcf9b54f3ba92 Mon Sep 17 00:00:00 2001 From: hudson-newey Date: Fri, 13 Sep 2024 11:24:22 +1000 Subject: [PATCH 01/50] Feat: Grid tiles automatically size to fill grid Fixes: #167 --- dev/use-cases/phil.html | 2 +- src/components/axes/axes.ts | 2 +- src/components/verification-grid/css/style.css | 5 +++++ src/components/verification-grid/verification-grid.ts | 7 +++++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/dev/use-cases/phil.html b/dev/use-cases/phil.html index 4909dd6b..d7f8b9d3 100644 --- a/dev/use-cases/phil.html +++ b/dev/use-cases/phil.html @@ -9,7 +9,7 @@
- + diff --git a/src/components/axes/axes.ts b/src/components/axes/axes.ts index 458761f0..ee092179 100644 --- a/src/components/axes/axes.ts +++ b/src/components/axes/axes.ts @@ -61,7 +61,7 @@ export class AxesComponent extends SignalWatcher(AbstractComponent(LitElement)) // label padding is the minimum additional distance between the labels // while the titleOffset is the distance between the axis title and the axis labels - private static labelPadding: EmUnit = 0.25; + private static labelPadding: EmUnit = 0.75; private static tickSize: EmUnit = 0.75; private static titleOffset: EmUnit = 0.25; diff --git a/src/components/verification-grid/css/style.css b/src/components/verification-grid/css/style.css index d1217527..a56a57a9 100644 --- a/src/components/verification-grid/css/style.css +++ b/src/components/verification-grid/css/style.css @@ -45,6 +45,7 @@ user-select: none; justify-content: center; align-items: stretch; + height: 70vh; /* We apply normal padding to the verification grid so that it is consistent @@ -62,6 +63,10 @@ gap: var(--oe-spacing); } +.grid-tile { + flex: 1 1; +} + .no-items-message { font-size: 1.2rem; } diff --git a/src/components/verification-grid/verification-grid.ts b/src/components/verification-grid/verification-grid.ts index fce63054..2eee4777 100644 --- a/src/components/verification-grid/verification-grid.ts +++ b/src/components/verification-grid/verification-grid.ts @@ -120,6 +120,12 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { @property({ attribute: "grid-size", type: Number, reflect: true }) public gridSize = 8; + @property({ attribute: "grid-size-n", type: Number }) + public gridSizeN = 5; + + @property({ attribute: "grid-size-m", type: Number }) + public gridSizeM = 4; + /** * The selection behavior of the verification grid * @values "desktop" | "tablet" | "default" @@ -1208,6 +1214,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { this.currentPage, (subject: SubjectWrapper, i: number) => html` Date: Fri, 13 Sep 2024 12:41:06 +1000 Subject: [PATCH 02/50] Fix axes --- src/components/axes/axes.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/components/axes/axes.ts b/src/components/axes/axes.ts index ee092179..b0a57a18 100644 --- a/src/components/axes/axes.ts +++ b/src/components/axes/axes.ts @@ -214,7 +214,7 @@ export class AxesComponent extends SignalWatcher(AbstractComponent(LitElement)) )}`; const yAxisGridLinesTemplate = svg`${yValues.map( - (value, i) => svg`${i > 0 && i < yValues.length - 1 ? yGridLineTemplate(value) : nothing}`, + (value, i) => svg`${i > 0 && i < yValues.length ? yGridLineTemplate(value) : nothing}`, )}`; return svg` @@ -249,9 +249,9 @@ export class AxesComponent extends SignalWatcher(AbstractComponent(LitElement)) scale(value)); - const positionDelta = lastTwoPositions[0] - lastTwoPositions[1]; - return positionDelta > fontSize + textLabelPadding; + const positionDelta = Math.abs(lastTwoPositions[0] - lastTwoPositions[1]); + + // we multiple the padding by two so that the padding is virtually applied + // to both labels in the axes + return positionDelta > fontSize + textLabelPadding * 2; } // the calculate step function will use a binary search to find the largest From b0da461723da66608c300a7cbd5d516f2b37f433 Mon Sep 17 00:00:00 2001 From: hudson-newey Date: Fri, 13 Sep 2024 14:27:16 +1000 Subject: [PATCH 03/50] Correct flexbox layout --- dev/use-cases/phil.html | 2 +- src/components/axes/axes.ts | 6 ++---- src/components/verification-grid/css/style.css | 3 +-- src/components/verification-grid/verification-grid.ts | 6 +++--- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/dev/use-cases/phil.html b/dev/use-cases/phil.html index d7f8b9d3..cc7eabbf 100644 --- a/dev/use-cases/phil.html +++ b/dev/use-cases/phil.html @@ -14,7 +14,7 @@ - +
diff --git a/src/components/axes/axes.ts b/src/components/axes/axes.ts index b0a57a18..43529280 100644 --- a/src/components/axes/axes.ts +++ b/src/components/axes/axes.ts @@ -59,6 +59,8 @@ import axesStyles from "./css/style.css?inline"; export class AxesComponent extends SignalWatcher(AbstractComponent(LitElement)) { public static styles = unsafeCSS(axesStyles); + public static fontCanvas: HTMLCanvasElement = document.createElement("canvas"); + // label padding is the minimum additional distance between the labels // while the titleOffset is the distance between the axis title and the axis labels private static labelPadding: EmUnit = 0.75; @@ -514,10 +516,6 @@ export class AxesComponent extends SignalWatcher(AbstractComponent(LitElement)) `; } - - // TODO: the canvas that we use to calculate the font width/height should be - // cached as a static field - public static fontCanvas: HTMLCanvasElement = document.createElement("canvas"); } declare global { diff --git a/src/components/verification-grid/css/style.css b/src/components/verification-grid/css/style.css index a56a57a9..ae4d1cd0 100644 --- a/src/components/verification-grid/css/style.css +++ b/src/components/verification-grid/css/style.css @@ -34,7 +34,6 @@ justify-content: center; align-items: stretch; background-color: var(--oe-background-color); - height: 100%; } .verification-grid { @@ -45,7 +44,7 @@ user-select: none; justify-content: center; align-items: stretch; - height: 70vh; + flex-basis: 100%; /* We apply normal padding to the verification grid so that it is consistent diff --git a/src/components/verification-grid/verification-grid.ts b/src/components/verification-grid/verification-grid.ts index 2eee4777..6d4a9ed4 100644 --- a/src/components/verification-grid/verification-grid.ts +++ b/src/components/verification-grid/verification-grid.ts @@ -1193,18 +1193,18 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) {
${when( this.currentPage.length === 0, From fd5d301b05283a96cb5defa81a245f395e8ae6e2 Mon Sep 17 00:00:00 2001 From: hudson-newey Date: Mon, 16 Sep 2024 10:34:23 +1000 Subject: [PATCH 04/50] Add dynamic grid size rounding --- src/components/spectrogram/css/style.css | 4 +- .../verification-grid-settings.ts | 4 +- .../verification-grid/css/style.css | 21 ++- .../verification-grid/verification-grid.ts | 135 +++++++++++++----- src/helpers/primes.ts | 13 ++ 5 files changed, 128 insertions(+), 49 deletions(-) create mode 100644 src/helpers/primes.ts diff --git a/src/components/spectrogram/css/style.css b/src/components/spectrogram/css/style.css index ee4735da..9eebe583 100644 --- a/src/components/spectrogram/css/style.css +++ b/src/components/spectrogram/css/style.css @@ -6,13 +6,13 @@ position: relative; background-color: var(--oe-panel-color-lighter); width: 100%; - height: auto; + height: 100%; } #spectrogram-container > canvas { position: relative; display: inherit; - height: auto; + height: 100%; /* We set the minimum height to 64 because the minimum FFT window size that diff --git a/src/components/verification-grid-settings/verification-grid-settings.ts b/src/components/verification-grid-settings/verification-grid-settings.ts index 423c66ba..651ebb85 100644 --- a/src/components/verification-grid-settings/verification-grid-settings.ts +++ b/src/components/verification-grid-settings/verification-grid-settings.ts @@ -66,7 +66,7 @@ export class VerificationGridSettingsComponent extends SignalWatcher(AbstractCom throw new Error("Could not find associated verification grid component"); } - this.gridSize = this.verificationGrid.gridSize; + this.gridSize = this.verificationGrid.targetGridSize; } private handleGridSizeChange(event: ChangeEvent) { @@ -78,7 +78,7 @@ export class VerificationGridSettingsComponent extends SignalWatcher(AbstractCom const inputValue = event.target.value; const newGridSize = Number(inputValue); - this.verificationGrid.gridSize = newGridSize; + this.verificationGrid.targetGridSize = newGridSize; this.updateGridSizeState(); } diff --git a/src/components/verification-grid/css/style.css b/src/components/verification-grid/css/style.css index ae4d1cd0..57bf96c4 100644 --- a/src/components/verification-grid/css/style.css +++ b/src/components/verification-grid/css/style.css @@ -28,6 +28,7 @@ padding: var(--oe-spacing); } +/* The container that contains both decision buttons and grid tiles */ .verification-container { display: flex; flex-wrap: wrap; @@ -36,15 +37,17 @@ background-color: var(--oe-background-color); } +/* The verification grid that contains verification grid tiles */ .verification-grid { - display: flex; - flex-wrap: wrap; - flex-grow: 1; - flex-basis: 100%; + --columns: 0; + --rows: 0; + + display: grid; + width: 100%; user-select: none; - justify-content: center; - align-items: stretch; - flex-basis: 100%; + + grid-template-columns: repeat(var(--columns), 1fr); + grid-template-rows: repeat(var(--rows), 1fr); /* We apply normal padding to the verification grid so that it is consistent @@ -62,10 +65,6 @@ gap: var(--oe-spacing); } -.grid-tile { - flex: 1 1; -} - .no-items-message { font-size: 1.2rem; } diff --git a/src/components/verification-grid/verification-grid.ts b/src/components/verification-grid/verification-grid.ts index 6d4a9ed4..2b31d8e8 100644 --- a/src/components/verification-grid/verification-grid.ts +++ b/src/components/verification-grid/verification-grid.ts @@ -1,6 +1,6 @@ import { customElement, property, query, queryAll, queryAssignedElements, state } from "lit/decorators.js"; import { AbstractComponent } from "../../mixins/abstractComponent"; -import { html, LitElement, nothing, PropertyValueMap, PropertyValues, TemplateResult, unsafeCSS } from "lit"; +import { html, LitElement, nothing, PropertyValueMap, TemplateResult, unsafeCSS } from "lit"; import { VerificationGridTileComponent } from "../verification-grid-tile/verification-grid-tile"; import { DecisionComponent, DecisionComponentUnion, DecisionEvent } from "../decision/decision"; import { VerificationHelpDialogComponent } from "./help-dialog"; @@ -24,6 +24,7 @@ import { when } from "lit/directives/when.js"; import { hasCtrlLikeModifier } from "../../helpers/userAgent"; import { decisionColor } from "../../services/colors"; import { ifDefined } from "lit/directives/if-defined.js"; +import { isPrime } from "../../helpers/primes"; import verificationGridStyles from "./css/style.css?inline"; export type SelectionObserverType = "desktop" | "tablet" | "default"; @@ -52,6 +53,12 @@ type SelectionEvent = CustomEvent<{ index: number; }>; +type GridShapeFactor = { + factorOne: number; + factorTwo: number; + targetDistance: number; +}; + // by keeping the elements position in a separate object, we can // avoid doing DOM queries every time we need to check if the element // is intersecting with the highlight box @@ -118,13 +125,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { /** The number of items to display in a single grid */ @property({ attribute: "grid-size", type: Number, reflect: true }) - public gridSize = 8; - - @property({ attribute: "grid-size-n", type: Number }) - public gridSizeN = 5; - - @property({ attribute: "grid-size-m", type: Number }) - public gridSizeM = 4; + public targetGridSize = 0; /** * The selection behavior of the verification grid @@ -174,6 +175,12 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { @state() public currentPage: SubjectWrapper[] = []; + @state() + private gridSizeN = this.targetGridSize; + + @state() + private gridSizeM = 1; + public get pagedItems(): number { return this.subjectHistory.length; } @@ -215,7 +222,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { /** A count of the number of tiles currently visible on the screen */ private get effectivePageSize(): number { - return this.gridSize - this.hiddenTiles; + return this.targetGridSize - this.hiddenTiles; } public connectedCallback(): void { @@ -257,32 +264,31 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { public firstUpdated(): void { this.gridContainer.addEventListener("selected", this.selectionHandler); this.decisionsContainer.addEventListener("decision", this.decisionHandler); - } - protected willUpdate(change: PropertyValues): void { - // whenever the gridSize property updates, we check that the new value - // is a finite, positive number. If it is not, then we cancel the change - // and revert to the old value - if (change.has("gridSize")) { - const oldGridSize = change.get("gridSize") as number; - const newGridSize = this.gridSize; + const newGridShape = this.computeGridShape(this.targetGridSize); + this.gridSizeN = newGridShape.factorOne; + this.gridSizeM = newGridShape.factorTwo; - // we use isFinite here to check that the value is not NaN, and that - // values such as Infinity are not considered as a valid grid size - if (!isFinite(newGridSize) || newGridSize <= 0) { - this.gridSize = oldGridSize; - console.error("New grid size value could not be converted to a number"); - } - } + this.doneFirstUpdate = true; } + private doneFirstUpdate = false; + protected async updated(change: PropertyValueMap): Promise { + if (this.doneFirstUpdate && change.has("targetGridSize")) { + const newTarget = this.targetGridSize; + + const newGridShape = this.computeGridShape(newTarget); + this.gridSizeN = newGridShape.factorOne; + this.gridSizeM = newGridShape.factorTwo; + } + const tileInvalidationKeys: (keyof this)[] = ["selectionBehavior"]; // gridSize is a part of source invalidation because if the grid size // increases, there will be verification grid tiles without any source // additionally, if the grid size is decreased, we want the "currentPage" // of sources to update / remove un-needed items - const sourceInvalidationKeys: (keyof this)[] = ["getPage", "gridSize"]; + const sourceInvalidationKeys: (keyof this)[] = ["getPage", "targetGridSize"]; // tile invalidations cause the functionality of the tiles to change // however, they do not cause the spectrograms or the template to render @@ -298,6 +304,66 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { } } + private computeGridShape(target: number): GridShapeFactor { + if (target === 1) { + return { factorOne: 1, factorTwo: 1, targetDistance: 0 }; + } + + while (isPrime(target)) { + target += 1; + } + + const targetContainer = this.gridContainer; + const targetContainerShape = targetContainer.getBoundingClientRect(); + // const targetContainerShape = window.screen; + + // e.g. 16/9 produces 1.77 + const targetAspectRatio = targetContainerShape.width / targetContainerShape.height; + + // some numbers e.g. 9 have one factor such as 3x3 which creates a aspect + // ratio of 1.00 + // using a 3x3 grid for a 16:9 screen is not ideal because it will create + // a lot of dead space + // therefore, we have a threshold that we have to meet. If we do not meet + // the threshold, we keep increasing the target until we find a grid shape + // that meets the threshold + const targetThreshold = 1.6; + + let optimalFactor: GridShapeFactor = { + factorOne: 1, + factorTwo: target, + targetDistance: Infinity, + }; + + // eslint-disable-next-line no-constant-condition + while (true) { + for (let i = 1; i <= target; i++) { + if (target % i === 0) { + const factorOne = i; + const factorTwo = target / i; + + // e.g. 1/2 produces 0.5 + // and 2/1 produces 2 + // so we should pick 2/1 in this case + const factorAspectRatio = factorOne / factorTwo; + const targetDistance = Math.abs(targetAspectRatio - factorAspectRatio); + + if (targetDistance < optimalFactor.targetDistance) { + optimalFactor = { factorOne, factorTwo, targetDistance }; + } + } + } + + if (optimalFactor.targetDistance > targetThreshold) { + target += 1; + } else { + break; + } + } + + return optimalFactor; + } + private handleTileInvalidation(): void { // if the user doesn't explicitly define a "selection-behavior" attribute // then we will infer the selection behavior based on the device type @@ -333,7 +399,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { if (this.getPage) { this.paginationFetcher = new GridPageFetcher(this.getPage); - this.currentPage = await this.paginationFetcher.getItems(this.gridSize); + this.currentPage = await this.paginationFetcher.getItems(this.targetGridSize); } } @@ -622,7 +688,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { // we check that the help dialog is not open so that the user doesn't // accidentally create a sub-selection (e.g. through keyboard shortcuts) // when they can't actually see the grid items - return this.gridSize > 1 && !this.isHelpDialogOpen(); + return this.targetGridSize > 1 && !this.isHelpDialogOpen(); } private isMobileDevice(): boolean { @@ -778,7 +844,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { private async handlePreviousPageClick(): Promise { if (this.canNavigatePrevious()) { - this.historyHead += this.gridSize; + this.historyHead += this.targetGridSize; await this.renderHistory(this.historyHead); } } @@ -791,14 +857,14 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { private async pageForwardHistory(): Promise { if (this.canNavigateNext()) { - this.historyHead -= this.gridSize; + this.historyHead -= this.targetGridSize; await this.renderHistory(this.historyHead); } } private async renderHistory(historyOffset: number) { const decisionStart = Math.max(0, this.subjectHistory.length - historyOffset); - const decisionEnd = Math.min(this.subjectHistory.length, decisionStart + this.gridSize); + const decisionEnd = Math.min(this.subjectHistory.length, decisionStart + this.targetGridSize); const decisionHistory = this.subjectHistory.slice(decisionStart, decisionEnd); this.historyMode(); @@ -869,7 +935,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { } private canNavigateNext(): boolean { - return this.historyHead > this.gridSize; + return this.historyHead > this.targetGridSize; } //#endregion @@ -1126,7 +1192,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { private decisionPromptTemplate() { const subSelection = this.currentSubSelection; const hasSubSelection = subSelection.length > 0; - const hasMultipleTiles = this.gridSize > 1; + const hasMultipleTiles = this.targetGridSize > 1; if (this.hasClassificationTask() && this.hasVerificationTask()) { return this.mixedTaskPromptTemplate(hasMultipleTiles, hasSubSelection); @@ -1193,8 +1259,8 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) {
@@ -1202,6 +1268,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) {
Date: Mon, 16 Sep 2024 14:56:42 +1000 Subject: [PATCH 05/50] Backfill unit tests for dynamic grid sizes --- dev/use-cases/nsw.html | 4 +- dev/use-cases/phil.html | 28 +--- src/components/spectrogram/css/style.css | 1 + .../verification-grid-settings.ts | 18 ++- .../verification-grid/verification-grid.ts | 118 +++++++++----- src/models/rendering.ts | 5 + src/tests/helpers.ts | 19 ++- .../verification-grid.e2e.fixture.ts | 31 +++- .../verification-grid.e2e.spec.ts | 149 +++++++++++++++--- 9 files changed, 277 insertions(+), 96 deletions(-) diff --git a/dev/use-cases/nsw.html b/dev/use-cases/nsw.html index 4f9fcf25..2b551049 100644 --- a/dev/use-cases/nsw.html +++ b/dev/use-cases/nsw.html @@ -9,7 +9,7 @@
- + @@ -17,7 +17,7 @@ - +
diff --git a/dev/use-cases/phil.html b/dev/use-cases/phil.html index cc7eabbf..797f58d9 100644 --- a/dev/use-cases/phil.html +++ b/dev/use-cases/phil.html @@ -8,28 +8,12 @@ -
- - - - + + + + - - -
- - + + diff --git a/src/components/spectrogram/css/style.css b/src/components/spectrogram/css/style.css index 9eebe583..a692a208 100644 --- a/src/components/spectrogram/css/style.css +++ b/src/components/spectrogram/css/style.css @@ -13,6 +13,7 @@ position: relative; display: inherit; height: 100%; + max-height: 45vh; /* We set the minimum height to 64 because the minimum FFT window size that diff --git a/src/components/verification-grid-settings/verification-grid-settings.ts b/src/components/verification-grid-settings/verification-grid-settings.ts index 651ebb85..42b5c6aa 100644 --- a/src/components/verification-grid-settings/verification-grid-settings.ts +++ b/src/components/verification-grid-settings/verification-grid-settings.ts @@ -1,6 +1,6 @@ import { customElement, state } from "lit/decorators.js"; import { AbstractComponent } from "../../mixins/abstractComponent"; -import { html, LitElement, unsafeCSS } from "lit"; +import { html, LitElement, nothing, unsafeCSS } from "lit"; import { VerificationGridComponent, verificationGridContext, @@ -88,6 +88,17 @@ export class VerificationGridSettingsComponent extends SignalWatcher(AbstractCom throw new Error("Could not find associated verification grid component"); } + const maximumGridSize = this.verificationGrid.maximumGridSize; + const minimumGridSize = 1; + const gridSizeStep = 1; + + // if the user cannot change the grid size because the maximum grid size is + // equal to the minimum grid size, we should not show the grid size controls + // because their screen size is unlikely to change. + if (maximumGridSize === minimumGridSize) { + return nothing; + } + return html` @@ -110,8 +121,9 @@ export class VerificationGridSettingsComponent extends SignalWatcher(AbstractCom @change="${this.handleGridSizeChange}" type="range" value="${ifDefined(this.gridSize)}" - min="1" - max="36" + min="${minimumGridSize}" + max="${maximumGridSize}" + step="${gridSizeStep}" /> diff --git a/src/components/verification-grid/verification-grid.ts b/src/components/verification-grid/verification-grid.ts index 2b31d8e8..46805295 100644 --- a/src/components/verification-grid/verification-grid.ts +++ b/src/components/verification-grid/verification-grid.ts @@ -25,6 +25,8 @@ import { hasCtrlLikeModifier } from "../../helpers/userAgent"; import { decisionColor } from "../../services/colors"; import { ifDefined } from "lit/directives/if-defined.js"; import { isPrime } from "../../helpers/primes"; +import { Pixel } from "../../models/unitConverters"; +import { GridShape } from "../../models/rendering"; import verificationGridStyles from "./css/style.css?inline"; export type SelectionObserverType = "desktop" | "tablet" | "default"; @@ -53,12 +55,6 @@ type SelectionEvent = CustomEvent<{ index: number; }>; -type GridShapeFactor = { - factorOne: number; - factorTwo: number; - targetDistance: number; -}; - // by keeping the elements position in a separate object, we can // avoid doing DOM queries every time we need to check if the element // is intersecting with the highlight box @@ -74,6 +70,15 @@ interface HighlightSelection { elements: IntersectionElement[]; } +interface BreakpointGridSize { + screenWidth: Pixel; + gridSize: number; +} + +interface GridShapeFactor extends GridShape { + targetDistance: number; +} + /** * @description * A verification grid component that can be used to verify audio events @@ -193,10 +198,10 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { private keydownHandler = this.handleKeyDown.bind(this); private keyupHandler = this.handleKeyUp.bind(this); private blurHandler = this.handleWindowBlur.bind(this); - private intersectionHandler = this.handleIntersection.bind(this); private selectionHandler = this.handleSelection.bind(this); private decisionHandler = this.handleDecision.bind(this); - private intersectionObserver = new IntersectionObserver(this.intersectionHandler); + private intersectionHandler = this.handleIntersection.bind(this); + private intersectionObserver = new IntersectionObserver(this.intersectionHandler, { threshold: 1 }); // the subject history contains all the subjects that have decisions applied // to them, while the verification buffer contains all the subjects that have @@ -217,14 +222,24 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { highlighting: false, elements: [], }; - private paginationFetcher?: GridPageFetcher; + private gridSizeBreakpoints: BreakpointGridSize[] = [ + // mobile devices + { screenWidth: 0, gridSize: 1 }, + + // desktop devices + { screenWidth: 720, gridSize: 9 }, + ]; /** A count of the number of tiles currently visible on the screen */ private get effectivePageSize(): number { return this.targetGridSize - this.hiddenTiles; } + public get maximumGridSize(): number { + return this.targetGridSize; + } + public connectedCallback(): void { super.connectedCallback(); document.addEventListener("keydown", this.keydownHandler); @@ -265,9 +280,25 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { this.gridContainer.addEventListener("selected", this.selectionHandler); this.decisionsContainer.addEventListener("decision", this.decisionHandler); - const newGridShape = this.computeGridShape(this.targetGridSize); - this.gridSizeN = newGridShape.factorOne; - this.gridSizeM = newGridShape.factorTwo; + // initial grid size < attributes < grid size settings < maximum grid size + if (this.targetGridSize) { + const newGridShape = this.computeGridShape(this.targetGridSize); + this.gridSizeN = newGridShape.columns; + this.gridSizeM = newGridShape.rows; + } else { + const screenSize = window.screen; + const relativeBreakpoint = this.gridSizeBreakpoints.findLast( + (breakpoint) => screenSize.width >= breakpoint.screenWidth, + ); + + if (relativeBreakpoint) { + this.targetGridSize = relativeBreakpoint.gridSize; + } else { + throw new Error("Could not find a suitable grid size for the current screen size"); + } + } + + this.intersectionObserver.observe(this.gridContainer); this.doneFirstUpdate = true; } @@ -279,8 +310,11 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { const newTarget = this.targetGridSize; const newGridShape = this.computeGridShape(newTarget); - this.gridSizeN = newGridShape.factorOne; - this.gridSizeM = newGridShape.factorTwo; + this.gridSizeN = newGridShape.columns; + this.gridSizeM = newGridShape.rows; + + // calculate if the new grid size will fit inside the container + this.handleResize(); } const tileInvalidationKeys: (keyof this)[] = ["selectionBehavior"]; @@ -306,7 +340,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { private computeGridShape(target: number): GridShapeFactor { if (target === 1) { - return { factorOne: 1, factorTwo: 1, targetDistance: 0 }; + return { columns: 1, rows: 1, targetDistance: 0 }; } while (isPrime(target)) { @@ -327,11 +361,11 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { // therefore, we have a threshold that we have to meet. If we do not meet // the threshold, we keep increasing the target until we find a grid shape // that meets the threshold - const targetThreshold = 1.6; + const targetThreshold = 2.0; let optimalFactor: GridShapeFactor = { - factorOne: 1, - factorTwo: target, + columns: 1, + rows: target, targetDistance: Infinity, }; @@ -339,25 +373,25 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { while (true) { for (let i = 1; i <= target; i++) { if (target % i === 0) { - const factorOne = i; - const factorTwo = target / i; + const columns = i; + const rows = target / i; // e.g. 1/2 produces 0.5 // and 2/1 produces 2 // so we should pick 2/1 in this case - const factorAspectRatio = factorOne / factorTwo; + const factorAspectRatio = columns / rows; const targetDistance = Math.abs(targetAspectRatio - factorAspectRatio); if (targetDistance < optimalFactor.targetDistance) { - optimalFactor = { factorOne, factorTwo, targetDistance }; + optimalFactor = { columns, rows, targetDistance }; } } } - if (optimalFactor.targetDistance > targetThreshold) { - target += 1; - } else { + if (optimalFactor.targetDistance < targetThreshold) { break; + } else { + target += 1; } } @@ -403,6 +437,21 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { } } + private handleIntersection(entries: IntersectionObserverEntry[]): void { + for (const entry of entries) { + if (entry.intersectionRatio < 1) { + if (this.targetGridSize > 1) { + this.targetGridSize -= 1; + } + } + } + } + + private handleResize() { + const intersectionRecords = this.intersectionObserver.takeRecords(); + this.handleIntersection(intersectionRecords); + } + private updateRequiredClassificationTags(): void { const requiredTags = new Map(); @@ -574,24 +623,6 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { this.showingSelectionShortcuts = false; } - // TODO: The intersection observer has been disabled because its functionality - // is buggy when the verification grid is larger than the screen size - // see: https://github.com/ecoacoustics/web-components/issues/146 - // see: https://github.com/ecoacoustics/web-components/issues/147 - // eslint-disable-next-line @typescript-eslint/no-unused-vars - private handleIntersection(_entries: IntersectionObserverEntry[]): void { - // for (const entry of entries) { - // if (entry.intersectionRatio < 1) { - // // at the very minimum, we always want one grid tile showing - // // even if this will cause some items to go off the screen - // const newProposedHiddenTiles = this.hiddenTiles + 1; - // if (newProposedHiddenTiles < this.gridSize) { - // this.hideGridItems(newProposedHiddenTiles); - // } - // } - // } - } - private handleSelection(selectionEvent: SelectionEvent): void { if (!this.canSubSelect()) { return; @@ -1272,6 +1303,7 @@ export class VerificationGridComponent extends AbstractComponent(LitElement) { @pointerdown="${this.renderHighlightBox}" @pointerup="${this.hideHighlightBox}" @pointermove="${this.resizeHighlightBox}" + @resize="${this.handleResize}" > ${when( this.currentPage.length === 0, diff --git a/src/models/rendering.ts b/src/models/rendering.ts index 9f30a721..b140ae9e 100644 --- a/src/models/rendering.ts +++ b/src/models/rendering.ts @@ -5,6 +5,11 @@ export interface Size { height: number; } +export interface GridShape { + rows: number; + columns: number; +} + // 2D slices are alwyas a subset of an fft spectrogram export class TwoDSlice { public constructor(data: TwoDSlice) { diff --git a/src/tests/helpers.ts b/src/tests/helpers.ts index 2f6a5d17..ce73c550 100644 --- a/src/tests/helpers.ts +++ b/src/tests/helpers.ts @@ -6,6 +6,13 @@ import { expect } from "./assertions"; import { Pixel } from "../models/unitConverters"; import { CssVariable } from "../helpers/types/advancedTypes"; +export type DeviceMock = + | typeof changeToMobile + | typeof changeToTabletPortrait + | typeof changeToTabletLandscape + | typeof changeToLaptop + | typeof changeToDesktop; + export async function getElementSize(element: T | Locator): Promise { const width = (await getBrowserValue(element, "clientWidth")) as number; const height = (await getBrowserValue(element, "clientHeight")) as number; @@ -17,11 +24,21 @@ export async function changeToMobile(page: Page) { await page.setViewportSize(mobileSize); } -export async function changeToTablet(page: Page) { +export async function changeToTabletPortrait(page: Page) { const tabletSize: Size = { width: 768, height: 1024 }; await page.setViewportSize(tabletSize); } +export async function changeToTabletLandscape(page: Page) { + const tabletLandscapeSize: Size = { width: 1024, height: 768 }; + await page.setViewportSize(tabletLandscapeSize); +} + +export async function changeToLaptop(page: Page) { + const laptopSize: Size = { width: 1366, height: 768 }; + await page.setViewportSize(laptopSize); +} + export async function changeToDesktop(page: Page) { const desktopSize: Size = { width: 1920, height: 1080 }; await page.setViewportSize(desktopSize); diff --git a/src/tests/verification-grid/verification-grid.e2e.fixture.ts b/src/tests/verification-grid/verification-grid.e2e.fixture.ts index 31bda12f..bb3c3725 100644 --- a/src/tests/verification-grid/verification-grid.e2e.fixture.ts +++ b/src/tests/verification-grid/verification-grid.e2e.fixture.ts @@ -8,6 +8,7 @@ import { getBrowserSignalValue, getBrowserValue, getCssColorVariable, + getCssVariable, invokeBrowserMethod, removeBrowserAttribute, setBrowserAttribute, @@ -18,7 +19,7 @@ import { VerificationGridComponent, VerificationGridSettings, } from "../../components/verification-grid/verification-grid"; -import { Size } from "../../models/rendering"; +import { GridShape, Size } from "../../models/rendering"; import { AxesComponent, DataSourceComponent, @@ -105,7 +106,7 @@ class TestPage { public async create(customTemplate = this.defaultTemplate) { await this.page.setContent(` - + ${customTemplate} { + const gridSize = await getBrowserValue(this.gridComponent(), "targetGridSize"); + return gridSize as number; + } + + public async getGridShape(): Promise { + const targetGrid = this.gridComponent(); + const columns = Number(await getCssVariable(targetGrid, "--columns")); + const rows = Number(await getCssVariable(targetGrid, "--rows")); + return { columns, rows }; + } + + public async getGridTileSize(): Promise { + // because all the grid tiles should be the same size, we can just check the + // first grid tile and get its size + const gridTiles = await this.gridTileComponents(); + const targetGridTile = gridTiles[0]; + + const boundingBox = await targetGridTile.boundingBox(); + if (!boundingBox) { + throw new Error("Grid tile bounding box not found"); + } + + return { width: boundingBox.width, height: boundingBox.height }; + } + // change attributes public async changeSelectionMode(mode: SelectionObserverType) { // we use "as any" here because the setBrowserAttribute helper typing checks diff --git a/src/tests/verification-grid/verification-grid.e2e.spec.ts b/src/tests/verification-grid/verification-grid.e2e.spec.ts index 8d3419c7..d3acb6c9 100644 --- a/src/tests/verification-grid/verification-grid.e2e.spec.ts +++ b/src/tests/verification-grid/verification-grid.e2e.spec.ts @@ -1,5 +1,15 @@ -import { Size } from "../../models/rendering"; -import { catchLocatorEvent, changeToDesktop, changeToMobile, getBrowserValue, setBrowserAttribute } from "../helpers"; +import { GridShape, Size } from "../../models/rendering"; +import { + catchLocatorEvent, + changeToDesktop, + changeToLaptop, + changeToMobile, + changeToTabletLandscape, + changeToTabletPortrait, + DeviceMock, + getBrowserValue, + setBrowserAttribute, +} from "../helpers"; import { verificationGridFixture as test } from "./verification-grid.e2e.fixture"; import { expect } from "../assertions"; import { @@ -56,8 +66,23 @@ test.describe("single verification grid", () => { }); test.describe("initial state", () => { - test("should have the correct grid size", async ({ fixture }) => { - const expectedGridSize = 3; + test("should have the correct grid size for a desktop screen size", async ({ fixture }) => { + await changeToDesktop(fixture.page); + const expectedGridSize = 9; + const gridSize = await fixture.getGridSize(); + expect(gridSize).toEqual(expectedGridSize); + }); + + test("should have the correct grid size for a tablet screen size", async ({ fixture }) => { + await changeToMobile(fixture.page); + const expectedGridSize = 1; + const gridSize = await fixture.getGridSize(); + expect(gridSize).toEqual(expectedGridSize); + }); + + test("should have the correct grid size for a mobile screen size", async ({ fixture }) => { + await changeToMobile(fixture.page); + const expectedGridSize = 1; const gridSize = await fixture.getGridSize(); expect(gridSize).toEqual(expectedGridSize); }); @@ -949,6 +974,84 @@ test.describe("single verification grid", () => { expect(realizedPagedItems).toBe(expectedPagedItems); }); + + test.describe("dynamic grid sizes", () => { + test("should add tiles if the number of tiles do not fill the verification grid", () => {}); + + test("should remove tiles if the tiles cause the verification grid to overflow", () => {}); + + test("should keep the same grid size if the user scrolls on the page", () => {}); + + interface DynamicGridSizeTest { + deviceName: string; + device: DeviceMock; + expectedGridSize: number; + expectedGridShape: GridShape; + expectedTileSize: Size; + } + + const testedGridSizes: DynamicGridSizeTest[] = [ + { + deviceName: "desktop", + device: changeToDesktop, + expectedGridSize: 10, + expectedGridShape: { rows: 5, columns: 2 }, + expectedTileSize: { width: 200, height: 200 }, + }, + { + deviceName: "laptop", + device: changeToLaptop, + expectedGridSize: 5, + expectedGridShape: { rows: 5, columns: 1 }, + expectedTileSize: { width: 200, height: 200 }, + }, + { + deviceName: "landscape tablet", + device: changeToTabletLandscape, + expectedGridSize: 3, + expectedGridShape: { rows: 3, columns: 1 }, + expectedTileSize: { width: 200, height: 200 }, + }, + { + deviceName: "portrait tablet", + device: changeToTabletPortrait, + expectedGridSize: 3, + expectedGridShape: { rows: 1, columns: 3 }, + expectedTileSize: { width: 200, height: 200 }, + }, + { + deviceName: "mobile", + device: changeToMobile, + expectedGridSize: 1, + expectedGridShape: { rows: 1, columns: 1 }, + expectedTileSize: { width: 200, height: 200 }, + }, + + // TODO: test Samsung Smart Fridge + ]; + + for (const testConfig of testedGridSizes) { + test.describe(testConfig.deviceName, () => { + test(`should have the correct target grid size`, async ({ fixture }) => { + await testConfig.device(fixture.page); + const realizedTargetGridSize = await fixture.getTargetGridSize(); + expect(realizedTargetGridSize).toEqual(testConfig.expectedGridSize); + }); + + test(`should have the correct grid shape`, async ({ fixture }) => { + await testConfig.device(fixture.page); + const realizedGridShape = await fixture.getGridShape(); + expect(realizedGridShape).toEqual(testConfig.expectedGridShape); + }); + + test(`should have the correct tile sizes`, async ({ fixture }) => { + await testConfig.device(fixture.page); + const realizedTileSize = await fixture.getGridTileSize(); + expect(realizedTileSize).toEqual(testConfig.expectedTileSize); + }); + }); + } + }); }); // during progressive creation, individual elements will be added to the @@ -960,10 +1063,10 @@ test.describe("decision meter", () => { test.describe("classification task", () => { test.beforeEach(async ({ fixture }) => { await fixture.create(` - - - - `); + + + + `); await fixture.dismissHelpDialog(); }); @@ -1089,9 +1192,9 @@ test.describe("decision meter", () => { test.describe("verification task", () => { test.beforeEach(async ({ fixture }) => { await fixture.create(` - - - `); + + + `); await fixture.dismissHelpDialog(); }); @@ -1124,13 +1227,13 @@ test.describe("decision meter", () => { test.describe("mixed classification and verification tasks", () => { test.beforeEach(async ({ fixture }) => { await fixture.create(` - Positive - Negative + Positive + Negative - Car - Bird - Cat - `); + Car + Bird + Cat + `); await fixture.dismissHelpDialog(); }); @@ -1172,13 +1275,13 @@ test.describe("verification grid with custom template", () => { test.describe("information cards", () => { test.beforeEach(async ({ fixture }) => { const customTemplate = ` - Koala - Not Koala + Koala + Not Koala - - `; + + `; await fixture.create(customTemplate); await fixture.changeGridSource(fixture.testJsonInput); From 684d15fdfc40909ef93ed2d63406ca04bac60a4a Mon Sep 17 00:00:00 2001 From: hudson-newey Date: Tue, 17 Sep 2024 07:44:19 +1000 Subject: [PATCH 06/50] Staging commit --- dev/use-cases/phil.html | 6 + .../verification-grid-settings.ts | 4 +- .../verification-grid/css/style.css | 26 ++-- .../verification-grid/verification-grid.ts | 121 ++++++++++-------- src/helpers/themes/theming.css | 1 - 5 files changed, 83 insertions(+), 75 deletions(-) diff --git a/dev/use-cases/phil.html b/dev/use-cases/phil.html index 797f58d9..c6cdef04 100644 --- a/dev/use-cases/phil.html +++ b/dev/use-cases/phil.html @@ -16,4 +16,10 @@ + + diff --git a/src/components/verification-grid-settings/verification-grid-settings.ts b/src/components/verification-grid-settings/verification-grid-settings.ts index 42b5c6aa..0ce03873 100644 --- a/src/components/verification-grid-settings/verification-grid-settings.ts +++ b/src/components/verification-grid-settings/verification-grid-settings.ts @@ -101,7 +101,7 @@ export class VerificationGridSettingsComponent extends SignalWatcher(AbstractCom return html` - +