feat: basic implementation for navigation among palette layers (resolves #5) #6

klown · 2024-02-15T19:28:19Z

This basic implementation uses a stack to keep track of the palette layers that the user has chosen. When a user activates a cell that leads to another layer, the palette containing the activated "go-to-layer" cell is pushed onto the stack. Whenever the user activates a "go-back" cell, the palette at top of the stack is popped off and navigated to.

Also, clean up index.html and move most of the functionality to index.js.

- renamed `ActionBranchToPalette` component to` ActionBranchToPaletteCell`, - adjusted file names, code, and palette json files as apprppriate for the name change.

netlify · 2024-02-15T19:28:35Z

✅ Deploy Preview for adaptive-palette ready!

Name	Link
🔨 Latest commit	`2e4c520`
🔍 Latest deploy log	https://app.netlify.com/sites/adaptive-palette/deploys/65f0a9a7df7bd20008b7b5f8
😎 Deploy Preview	https://deploy-preview-6--adaptive-palette.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

See: https://github.com/rollup/plugins/tree/master/packages/dynamic-import-vars#limitations

cindyli · 2024-02-21T16:13:19Z

src/ActionBranchToPaletteCell.test.ts

+import { initAdaptivePaletteGlobals } from "./GlobalData";
+import { ActionBranchToPaletteCell } from "./ActionBranchToPaletteCell";
+
+describe("ActionBranchToPaletteCell render tests", () => {


It would be nice to also test the click event of the ActionBranchToPaletteCell that leads to the rendering of the palette at the next layer.

Yes, and I decided that the best place to do that is in Palette.integration.test since the go-to button affects other areas of the screen and involves other objects.

cindyli · 2024-02-21T16:18:32Z

src/CommandGoBackCell.test.ts

+import { initAdaptivePaletteGlobals } from "./GlobalData";
+import { CommandGoBackCell } from "./CommandGoBackCell";
+
+describe("ActionBmwCodeDell render tests", () => {


Would be nice to test the click event of the go back button. Since this test involves paletteStore and navigationStack, it may fit better in the integration test file Palette.integration.test.ts?

The same idea applies in the comment above about testing the click event for ActionBranchToPaletteCell.

As above with the ActionBranchToPaletteCell, the go-back button test is in Palette.integration.test.

cindyli · 2024-02-21T16:20:03Z

src/ActionBranchToPaletteCell.ts

+// TODO:  this is identical to `ActionBmwCodeCellPropsType`.  Should it be?
+type ActionBranchToPalettePropsType = {


I'm thinking if we should expand the options section for every type component as this part is not defined and verified by typescript. What do you think?

I agree, and will start work on it next. I also noticed that we don't define the return type of many of our functions. Should we?

It's a great idea to define the return type for functions.

I've done them all. The one that was problematic was the return type for the htm() template function used by all of the component Functions. I thought htm returned a string type, but it does not. Its return type is something like TemplateStringsArray, but when I looked into the project's repository, I found issue #73, Question: How well does this work with TypeScript?. The main answer is given in the first response: "there aren't typings for htm / html, but there are extremely basic types for the react and preact integrations." The rest of the discussion is about TypeScript's effort to support a tagged template type. That work is ongoing.

I did some more debugging and it looks like, in the case of Preact, the htm return type is VNode, or "virtual DOM node". I'm currently using that for the return type.

As for the expanding the options, I'm still working on that.

I'm thinking if we should expand the options section for every type component as this part is not defined and verified by typescript. What do you think?

For "expanding the options section", I took the full BlissSymbolCellType and created new types from its parts. Since these are not palette cell types, I used "Info" in their type names; for example, a LayoutInfoType that contains only the grid-styles information. Then, one can define cell types by combining the info types as needed.

Let me know what you think.

cindyli · 2024-02-21T18:55:41Z

src/GlobalData.ts

+ * @return {JsonPaletteType} - The palette itself, or `null` if it could not be
+ *                             loaded.
+ */
+export async function importPaletteFromJsonFile (jsonFile: string, path: string) {


Would this function better belong to GlobalUtils.ts?

Yes, that makes sense. I moved it there.

cindyli · 2024-02-21T19:01:01Z

src/NavigationStack.ts

+
+  /**
+   * Return the palette at the top of the stack without changing the stack
+   * tself.  If an index is given, the palette at that index is returned.  Note


typo: "tself -> itself".

Another question is, if an index of zero denotes the top of the stack, what if someone wants to peek the palette at the bottom of the stack that actually uses the index zero?

I added a peekLast() function to look at the bottom of the stack. I first thought that a special index value such as -1 defined as the constant LAST_INDEX could be used, e.g., const palette = navStack.peek(LAST_INDEX), but that felt less transparent than a function for peeking at the bottom of the stack. So I switched to peekLast().

cindyli · 2024-02-21T19:22:14Z

src/index.js

+
+adaptivePaletteGlobals.navigationStack.currentPalette = firstLayer;
+render(html`<${Palette} json=${goBackCell} />`, document.getElementById("backup_palette"));
+//render(html`<${Palette} json=${outputPalette} />`, document.getElementById("output_palette"));


This commented line can be removed? Same as other related spots: line 29 in this file and this div container in index.html?

Right. All of them have been removed.

cindyli · 2024-02-21T20:02:02Z

When loading the palette page in a browser, the console log reports this error:

TypeError: bciToBlissaryId(...) is undefined
    bciAvIdToString SvgUtils.ts:27
    getSvgBuilder SvgUtils.ts:56
    getSvgElement SvgUtils.ts:91
    BlissSymbol BlissSymbol.ts:31
    Preact 19
    <anonymous> index.js:35
SvgUtils.ts:60:12

Unknown bci-av-id = 1 SvgUtils.ts:61:12

The error might come from this line.

…tion - initialize the globals with an optional id (of the main palette area), - set the `aria-controls` attribute of all go-back buttons to this id, - when going back use the `aria-controls` to find the container element to render the palette into.

…alUtils.ts`

klown · 2024-03-01T21:43:05Z

When loading the palette page in a browser, the console log reports this error:

TypeError: bciToBlissaryId(...) is undefined
    bciAvIdToString SvgUtils.ts:27
    getSvgBuilder SvgUtils.ts:56
    getSvgElement SvgUtils.ts:91
    BlissSymbol BlissSymbol.ts:31
    Preact 19
    <anonymous> index.js:35
SvgUtils.ts:60:12

Unknown bci-av-id = 1 SvgUtils.ts:61:12

The error might come from this line.

The error does come from that line. As far as I can tell, "BMW" does not have a blissary id nor Blissymbol. If it does, let me know.

But this points to another issue: Since the BlissSymbolCellType requires a bciAvId, some id has to be passed and in this case I chose a value that would not result in any symbol. And that causes the TypeError.

Should the bciAvId field of the BlissSymbolCellType be optional? Then the BlissSymbol component could check and if missing, make no attempt to get a Blissymbol. That is, the blissaryIdMap is not consulted, nor is the BlissSVGBuilder asked for a symbol that cannot exist.

klown · 2024-03-01T21:53:58Z

@cindyli Ready for another round of review.

I decided to change the click handlers for both the ActionBranchToPaletteCell and CommandGoBackCell: to remove the hard-coded document.getElementById("mainPaletteDisplayArea") that finds the container to render the new palette into. Instead I set that in the adaptivePaletteGlobals structure. In addition, when a CommandGoBackCell is rendered, its aria-controls attribute is set to that container element and is used by its click handler to pass to the renderer. The aria-controls is a nice way to store the information and it adds some accessibility information as well. I think we should add aria-controls to the CommandClearEncoding and the CommandDelLastEncoding elements for accessibility purposes where they point to the ContentBmwEncoding element. What do you think? (Link to info about aria-controls).

cindyli · 2024-03-04T19:10:52Z

Should the bciAvId field of the BlissSymbolCellType be optional? Then the BlissSymbol component could check and if missing, make no attempt to get a Blissymbol. That is, the blissaryIdMap is not consulted, nor is the BlissSVGBuilder asked for a symbol that cannot exist.

I feel if there isn't a value for the bciAvId field, this cell probably should not be a BlissSymbolCellType, which meant to display a bliss symbol.

The cell that has bciAvId==1 is the button that leads to the BMW palette. Do you think if it's a good idea to display bliss symbols of "B"(8552), "M"(8563), "W"(8573), so the bciAvId would be [8552, "/", 6563, "/", 8573]?

cindyli · 2024-03-04T19:39:56Z

I think we should add aria-controls to the CommandClearEncoding and the CommandDelLastEncoding elements for accessibility purposes where they point to the ContentBmwEncoding element. What do you think?

I think adding aria-controls is a great idea too. Are you going to add for the CommandClearEncoding and the CommandDelLastEncoding? Let me know if you want me to add. Thanks.

…utton Also tweak the label of the "Famliy" button and the bliss symbol "People" of the button.

klown · 2024-03-11T19:02:30Z

Should the bciAvId field of the BlissSymbolCellType be optional? Then the BlissSymbol component could check and if missing, make no attempt to get a Blissymbol. That is, the blissaryIdMap is not consulted, nor is the BlissSVGBuilder asked for a symbol that cannot exist.

I feel if there isn't a value for the bciAvId field, this cell probably should not be a BlissSymbolCellType, which meant to display a bliss symbol.

The cell that has bciAvId==1 is the button that leads to the BMW palette. Do you think if it's a good idea to display bliss symbols of "B"(8552), "M"(8563), "W"(8573), so the bciAvId would be [8552, "/", 6563, "/", 8573]?

You meant ``[8552, "/", 8563, "/", 8573]` :-).

After thinking about it, I agree that a BlissSymbolCellType must have a Bliss symbol, otherwise, it's a different type of cell. Your solution works for me, although it looks a little odd when rendered since the Bliss symbol "BMW" looks similar to the English label text. But, that's okay.

klown · 2024-03-11T19:03:14Z

I think we should add aria-controls to the CommandClearEncoding and the CommandDelLastEncoding elements for accessibility purposes where they point to the ContentBmwEncoding element. What do you think?

I think adding aria-controls is a great idea too. Are you going to add for the CommandClearEncoding and the CommandDelLastEncoding? Let me know if you want me to add. Thanks.

I came up with a solution that adds another property in the .json palette definition called ariaControls. Its value is the id of the cell it controls. So far, that cell and its id are in the same .json file. The cell's id is already copied into the DOM element's @id attribute. It's easy to set an @aria-controls attribute to refer to the appropriate DOM element's @id.

Let me know what you think.

cindyli · 2024-03-12T19:56:30Z

src/index.d.ts

 };

+export type BlissSymbolCellType = LayoutInfoType & BranchToInfoType & BlissSymbolInfoType;


Great finding of this useful feature.

klown · 2024-03-13T20:42:54Z

Ready for another review, @cindyli.

klown added 27 commits October 30, 2023 16:09

feat: select from a set of palettes

7105026

fix: palette json definition files for "people" and "myfamily"

6f25a5f

Merge branch 'feat/create-palette-from-json' into MultiPalettes

0adab7b

feat: added main "palette of palettes" for choosing other palettes

f7530a8

Merge branch 'feat/create-palette-from-json' into MultiPalettes

f01e7e0

fix: actually add the new ActionBranchToPalette component

094f434

Merge branch 'feat/create-palette-from-json' into MultiPalettes

e43465c

Merge branch 'feat/create-palette-from-json' into MultiPalettes

5e3743f

Merge branch 'feat/create-palette-from-json' into MultiPalettes

4752e51

Merge branch 'main' into MultiPalettes

4305b13

feat: load palette json files when clicking on palette selection cells

d5df467

chore: add note about "singing" palette cell as a test

10a95a8

Merge branch 'feat/create-bmw-encoding' into multipaletteAndOutput

30b570a

chore: modify to work with branch feat/create-bmw-encoding

4c91168

fix: use "isPresentational" for the symbols in the content display area

10e33c5

chore: modfiy palette json to expand to fit the display

a73e48a

Also, clean up index.html and move most of the functionality to index.js.

Merge branch 'feat/create-bmw-encoding' into multipaletteAndOutput

7201c22

feat: implement basic navigation/layer tracking with back button

9555c79

Merge branch 'feat/create-bmw-encoding' into multipaletteAndOutput

0357a3c

fix: change style class of "back button" to be a command style

15b899f

Merge branch 'main' into multipaletteAndOutput

4594254

feat: unit tests for basic navigation/layer tracking

a7c6d83

feat: implement navigation by layers (demo)

8776a1b

feat: implement lazy loading of palettes as needed

82d7cd6

fix: add style file for ActionBranchToPalette component

08d262c

fix: make names of cells consistent

1ea48b5

- renamed `ActionBranchToPalette` component to` ActionBranchToPaletteCell`, - adjusted file names, code, and palette json files as apprppriate for the name change.

feat: test that back button can be in verious locations

de1028a

klown added 2 commits February 20, 2024 09:53

fix: replace fetch() with import() for accessing local files.

84ca4f6

fix: implement dynamic import() based on restrictions

946ee4e

See: https://github.com/rollup/plugins/tree/master/packages/dynamic-import-vars#limitations

cindyli requested changes Feb 21, 2024

View reviewed changes

klown added 5 commits February 26, 2024 11:16

feat: implement the ability to peek at the bottom of the NavigationStack

44c960c

feat: add tests of the handlers for "go-to" and "go-back" cells

b30c693

fix: move importPaletteFromJsonFile() from GlobalData.ts to `Glob…

3b832fd

…alUtils.ts`

fix: format a comment properly

7380960

klown added 4 commits March 6, 2024 10:21

fix: use bliss letters "BMW" for bliss symbol for BWM go-to-palette b…

4f7cc54

…utton Also tweak the label of the "Famliy" button and the bliss symbol "People" of the button.

feat: define return types for all functions

49a10b0

feat: implement aria-controls for clear and delete cells

4e3a17f

feat: rename "keyboards" folder to "palettes"

13b87f7

feat: decompose types into reusable parts and combine as appropriate

2e4c520

cindyli reviewed Mar 12, 2024

View reviewed changes

cindyli approved these changes Mar 14, 2024

View reviewed changes

cindyli merged commit 100b474 into inclusive-design:main Mar 14, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: basic implementation for navigation among palette layers (resolves #5) #6

feat: basic implementation for navigation among palette layers (resolves #5) #6

klown commented Feb 15, 2024

netlify bot commented Feb 15, 2024 •

edited

Loading

cindyli Feb 21, 2024

klown Mar 1, 2024

cindyli Feb 21, 2024

klown Mar 1, 2024

cindyli Feb 21, 2024

klown Mar 1, 2024

cindyli Mar 4, 2024

klown Mar 11, 2024

klown Mar 12, 2024

cindyli Feb 21, 2024

klown Mar 1, 2024

cindyli Feb 21, 2024

klown Mar 1, 2024

cindyli Feb 21, 2024

klown Mar 1, 2024

cindyli commented Feb 21, 2024

klown commented Mar 1, 2024

klown commented Mar 1, 2024

cindyli commented Mar 4, 2024

cindyli commented Mar 4, 2024

klown commented Mar 11, 2024

klown commented Mar 11, 2024

cindyli Mar 12, 2024

klown commented Mar 13, 2024

		// TODO: this is identical to `ActionBmwCodeCellPropsType`. Should it be?
		type ActionBranchToPalettePropsType = {

		};

		export type BlissSymbolCellType = LayoutInfoType & BranchToInfoType & BlissSymbolInfoType;

feat: basic implementation for navigation among palette layers (resolves #5) #6

feat: basic implementation for navigation among palette layers (resolves #5) #6

Conversation

klown commented Feb 15, 2024

netlify bot commented Feb 15, 2024 • edited Loading

✅ Deploy Preview for adaptive-palette ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindyli commented Feb 21, 2024

klown commented Mar 1, 2024

klown commented Mar 1, 2024

cindyli commented Mar 4, 2024

cindyli commented Mar 4, 2024

klown commented Mar 11, 2024

klown commented Mar 11, 2024

Choose a reason for hiding this comment

klown commented Mar 13, 2024

netlify bot commented Feb 15, 2024 •

edited

Loading