-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Create palette from JSON file (resolves #1) #2
Changes from 9 commits
43ba41e
2485833
f1ac9e0
9542e87
808b7d8
47ab98c
3906b19
25ab172
98aecfb
a1a318d
7a2c857
126de89
d3db085
3592783
ea5e80c
a0080ca
a58d0fb
58ed03d
43eafe7
77c240e
6be1737
fc594f6
1088f9e
6a32931
4b05f39
fd3f054
0b54137
fdb3d67
8b7f61d
f2b4048
0639ece
406d52f
cb4afb8
3cbdbee
278a67e
bc98779
106fb01
7900134
380dd91
b398c6b
0a97b2a
f5e7136
da78a8b
36f197e
2819b29
8118cd7
0c73c0d
d75020b
4c20fcd
eeb4e77
e252bea
7dfdf63
184498a
8038d14
dd57a24
d0d5237
cabf34d
352ce6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,15 @@ | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<meta name="color-scheme" content="light dark" /> | ||
<title>Adaptive Palette</title> | ||
</head> | ||
<body> | ||
<div id="app"></div> | ||
</body> | ||
<head> | ||
<meta charset="UTF-8" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<meta name="color-scheme" content="light dark" /> | ||
<title>Adaptive Palette</title> | ||
</head> | ||
<body> | ||
<h2>Palette Based on JSON</h1> | ||
<div id="paletteCell"> | ||
<script type="module" src="/src/Palette.ts"></script> | ||
</div> | ||
</body> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* | ||
* Copyright 2023 Inclusive Design Research Centre, OCAD University | ||
* All rights reserved. | ||
* | ||
* Licensed under the New BSD license. You may not use this file except in | ||
* compliance with this License. | ||
* | ||
* You may obtain a copy of the License at | ||
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE | ||
*/ | ||
|
||
.paletteContainer { | ||
display: grid; | ||
grid-template-columns: auto auto auto; | ||
border: 2px solid #f76707; | ||
border-radius: 5px; | ||
background-color: pink; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
/* | ||
* Copyright 2023 Inclusive Design Research Centre, OCAD University | ||
* All rights reserved. | ||
* | ||
* Licensed under the New BSD license. You may not use this file except in | ||
* compliance with this License. | ||
* | ||
* You may obtain a copy of the License at | ||
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE | ||
*/ | ||
|
||
import { render } from "preact"; | ||
import { html } from "htm/preact"; | ||
import { PaletteCell } from "./PaletteCell"; | ||
import "./Palette.scss"; | ||
|
||
/** | ||
* Given a palette defined in a json structure, compute the number of rows | ||
* and columns in that palette. | ||
* | ||
* @param {Object} paletteDefinition - An object that lists the positions, | ||
* heights and widths of the cells in the palette. | ||
* @return {Object} - The row and column counts: `{ numRows: ..., numColumns: ...}`. | ||
*/ | ||
function countRowsColumns (paletteDefinition) { | ||
let rowCount = 0; | ||
let colCount = 0; | ||
let rightColumn = 0; | ||
let bottomRow = 0; | ||
const cellIds = Object.keys(paletteDefinition.cells); | ||
cellIds.forEach((id) => { | ||
const cellOptions = paletteDefinition.cells[id].options; | ||
rightColumn = cellOptions.columnStart + cellOptions.columnSpan; | ||
if (rightColumn > colCount) { | ||
colCount = rightColumn; | ||
} | ||
bottomRow = cellOptions.rowStart + cellOptions.rowSpan; | ||
if (bottomRow > rowCount) { | ||
rowCount = bottomRow; | ||
} | ||
}); | ||
return { numRows: rowCount, numColumns: colCount }; | ||
} | ||
|
||
export function Palette (props) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A typescript The same comment applies to other component props. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was going to write an interface for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, but only for the Palette component itself. It would be good to test the There is a simple solution given in a stackoverflow article. The solution is to create and export a structure named
It's less of a hack. Additional searching led to an article about the subject and from there to a plugin called |
||
|
||
const paletteDefinition = props.json; | ||
const rowsCols = countRowsColumns(paletteDefinition); | ||
const cellIds = Object.keys(paletteDefinition.cells); | ||
|
||
// Loop to create an array of renderings for each cell | ||
const theCells = []; | ||
cellIds.forEach((id) => { | ||
const aCell = paletteDefinition.cells[id]; | ||
const cellOptions = aCell.options; | ||
const paletteCell = html` | ||
<${PaletteCell} id="${id}" labelText="${cellOptions.label}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. I've also incorporated your latest
At the moment, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replying to myself: I'm at the wrong level. If there is a registry, the type will be used by the
There is probably no reason to pass the |
||
columnStart="${cellOptions.columnStart}" columnSpan="${cellOptions.columnSpan}" | ||
rowStart="${cellOptions.rowStart}" rowSpan="${cellOptions.rowSpan}" | ||
/>`; | ||
theCells.push(paletteCell); | ||
}); | ||
|
||
return html` | ||
<div | ||
class="paletteContainer" | ||
style="grid-template-columns: repeat(${rowsCols.numColumns}, auto);"> | ||
${theCells} | ||
</div> | ||
`; | ||
} | ||
|
||
import bmwJson from "./keyboards/bmw_palette.json"; | ||
render (html`<${Palette} json=${bmwJson}/>`, document.getElementById("paletteCell")); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* Copyright 2023 Inclusive Design Research Centre, OCAD University | ||
* All rights reserved. | ||
* | ||
* Licensed under the New BSD license. You may not use this file except in | ||
* compliance with this License. | ||
* | ||
* You may obtain a copy of the License at | ||
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE | ||
*/ | ||
|
||
.paletteCell { | ||
border: 2px solid blue; | ||
background-color: gray; | ||
border-radius: 5px; | ||
padding: 1em; | ||
font-size: 1em; | ||
text-align: center; | ||
|
||
&:hover, &:focus { | ||
background-color: lightblue; | ||
color: #0000aa; | ||
} | ||
|
||
& > svg { | ||
display: block; | ||
} | ||
} | ||
|
||
.disabled { | ||
opacity: 0.6; | ||
color: #090909; | ||
cursor: not-allowed; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
/* | ||
* Copyright 2023 Inclusive Design Research Centre, OCAD University | ||
* All rights reserved. | ||
* | ||
* Licensed under the New BSD license. You may not use this file except in | ||
* compliance with this License. | ||
* | ||
* You may obtain a copy of the License at | ||
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE | ||
*/ | ||
|
||
import { render, screen, fireEvent } from "@testing-library/preact"; | ||
import "@testing-library/jest-dom"; | ||
import { html } from "htm/preact"; | ||
import { PaletteCell } from "./PaletteCell"; | ||
|
||
test("The PaletteCell is rendered correctly", async () => { | ||
render(html` | ||
<${PaletteCell} | ||
id="uuid-of-some-kind" | ||
labelText="Bliss Language" | ||
rowStart="3" | ||
rowSpan="2" | ||
columnStart="2" | ||
columnSpan="1" | ||
style="background-color: green;" | ||
/>`); | ||
|
||
let button = await screen.findByRole("button", {name: "Bliss Language"}); | ||
let buttonStyles = window.getComputedStyle(button); | ||
console.log(`Button's background colour: ${buttonStyles.backgroundColor}`); | ||
|
||
// Check that the PaletteCell/button is rendered and has the correct | ||
// attributes and text | ||
expect(button).toBeVisible(); | ||
expect(button).toBeValid(); | ||
expect(button.id).toBe("uuid-of-some-kind"); | ||
|
||
// PaletteCell.css does not specify a bacground colour. The background will | ||
// be whatever the browser default is. For now, check that the specified | ||
// colour in the test render above is correct | ||
expect(button.style["background-color"]).toBe("green"); | ||
expect(button.style["grid-column"]).toBe("2 / span 1"); | ||
expect(button.style["grid-row"]).toBe("3 / span 2"); | ||
expect(button.textContent).toBe("Bliss Language"); | ||
|
||
// Check disabled state (should be enabled) | ||
expect(button.getAttribute("disabled")).toBe(null); | ||
expect(button.getAttribute("class")).not.toContain("disabled"); | ||
|
||
// Check styling due to mouse hover | ||
// TODO: get `:hover` style from PaletteCell.css? | ||
fireEvent.mouseOver(button); | ||
|
||
buttonStyles = window.getComputedStyle(button); | ||
console.log(`Button's background colour: ${buttonStyles.backgroundColor}`); | ||
//expect(buttonStyles.backgroundColor).toBe("#cccccc"); | ||
//expect(buttonStyles.color).toBe("#0000aa"); | ||
|
||
fireEvent.mouseLeave(button); | ||
buttonStyles = window.getComputedStyle(button); | ||
console.log(`Button's background colour: ${buttonStyles.backgroundColor}`); | ||
//expect(buttonStyles.backgroundColor).not.toBe("#cccccc"); | ||
//expect(buttonStyles.color).not.toBe("#0000aa"); | ||
|
||
// Create a diabled button and check its disabled state. | ||
render(html` | ||
<${PaletteCell} | ||
id="uuid-of-another-kind" | ||
labelText="Disabled Cell" | ||
rowStart="3" | ||
rowSpan="2" | ||
columnStart="2" | ||
columnSpan="1" | ||
class="disabled" | ||
/>`); | ||
|
||
button = await screen.findByRole("button", {name: "Disabled Cell"}); | ||
expect(button).toBeVisible(); | ||
expect(button).toBeValid(); | ||
expect(button.id).toBe("uuid-of-another-kind"); | ||
expect(button.getAttribute("disabled")).toBe(""); | ||
expect(button.getAttribute("class")).toContain("disabled"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* Copyright 2023 Inclusive Design Research Centre, OCAD University | ||
* All rights reserved. | ||
* | ||
* Licensed under the New BSD license. You may not use this file except in | ||
* compliance with this License. | ||
* | ||
* You may obtain a copy of the License at | ||
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE | ||
*/ | ||
|
||
import { html } from "htm/preact"; | ||
import "./PaletteCell.scss"; | ||
|
||
export function PaletteCell (props) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding the component name, I was thinking it uses the same name as the cell type it supports. The name starts with a more general category performed by the cell. For example, keys that perform actions when pressed are in "action" category, so this cell type and component for BMW code keys are named "ActionBmwCodeKeys"; names for command buttons start with "Command..."; names for display areas start with "Content...". There might be better names than "action", "command" and "content". This naming convention will make files in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea, especially the way that files are grouped within the Still, there is nothing within "Insert" to communicate the idea of entering a sequence of codes. Then again, I'm not sure there needs to be. |
||
|
||
// Basic styles are the `paletteCell` class defined in PaletteCell.css. | ||
// Concatenate any additional classes provided by `props`. | ||
let classes = "paletteCell"; | ||
if (props.class) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There isn't yet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say it was too early to remove it :-) I am using an extra
That is handled within
However, having said that, I don't know how disabled is actually handled on the palette. We know that as bmw codes are entered, certain keys are disabled since they are irrelevant for the current code sequence. How is that handled by Preact? This is a case of tracking state and probably involves hooks but I do not know the details. I will have a look at the documentation. |
||
classes = `${classes} ${props.class}`; | ||
} | ||
let disabled = false; | ||
if (classes.indexOf("disabled") >= 0) { | ||
disabled = true; | ||
} | ||
|
||
// Also concatenate local styles with given grid cell styles | ||
let styles = ` | ||
grid-column: ${props.columnStart} / span ${props.columnSpan}; | ||
grid-row: ${props.rowStart} / span ${props.rowSpan}; | ||
`; | ||
if (props.style) { | ||
styles = `${styles} ${props.style}`; | ||
} | ||
|
||
// Placeholder for svg graphic for the cell | ||
const svgString = html` | ||
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10" role="presentation"> | ||
<circle cx="5" cy="5" r="4" fill="transparent" stroke="black" stroke-width="1"/> | ||
</svg> | ||
`; | ||
|
||
return html` | ||
<button id="${props.id}" class="${classes}" style="${styles}" disabled=${disabled}> | ||
${svgString} | ||
${props.labelText} | ||
</button> | ||
`; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/* | ||
* Copyright 2023 Inclusive Design Research Centre, OCAD University | ||
* All rights reserved. | ||
* | ||
* Licensed under the New BSD license. You may not use this file except in | ||
* compliance with this License. | ||
* | ||
* You may obtain a copy of the License at | ||
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE | ||
*/ | ||
|
||
"use strict"; | ||
|
||
import { PaletteStore } from "./PaletteStore"; | ||
|
||
describe("PaletteStore module", () => { | ||
|
||
const dummyPalette1 = { name: "dummyPalette1", palette: null }; | ||
const dummyPalette2 = { name: "dummyPalette2", palette: null }; | ||
const dummyPalette2Name = "DummyPalette2"; | ||
|
||
const paletteStore = new PaletteStore(); | ||
|
||
test("Empty PaletteStore", () => { | ||
expect(paletteStore.isEmpty()).toBe(true); | ||
}); | ||
|
||
test("Non-empty PaletteStore", () => { | ||
paletteStore.addPalette(dummyPalette1); | ||
expect(paletteStore.isEmpty()).toBe(false); | ||
expect(paletteStore.numPalettes).toBe(1); | ||
expect(paletteStore.paletteList).toEqual(["dummyPalette1"]); | ||
}); | ||
|
||
test("Add another palette", () => { | ||
paletteStore.addPalette(dummyPalette2, dummyPalette2Name); | ||
expect(paletteStore.numPalettes).toBe(2); | ||
expect(paletteStore.paletteList).toEqual(["dummyPalette1", dummyPalette2Name]); | ||
}); | ||
|
||
test("Retrieve a palette", () => { | ||
const retrievedPalette = paletteStore.getNamedPalette(dummyPalette2Name); | ||
expect(retrievedPalette).toBe(dummyPalette2); | ||
}); | ||
|
||
test("Delete a palette", () => { | ||
const numPalettes = paletteStore.numPalettes; | ||
const removedPalette = paletteStore.removePalette(dummyPalette1.name); | ||
expect(removedPalette).toBe(dummyPalette1); | ||
expect(paletteStore.numPalettes).toBe(numPalettes - 1); | ||
expect(paletteStore.getNamedPalette(dummyPalette1.name)).toBeUndefined(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This div is used to render the entire palette. It might be better to use an id "palette".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
paletteCell
id was left over from when the code only rendered a singlePaletteCell
. I have changed it tobmwKeyCodes
for now, but I would not be surprised if it's changed again.I also cleaned up
Palette.ts
by removing the rendering code and moving it toindex.html
.