0.35.1 fix keyboard navigation

CHANGELOG.md

@@ -4,6 +4,12 @@ Note: Many earlier versions are not specified, that's too much work.
 
 When a `@types` dependency updates, they almost always don't affect anything.
 
+## v0.35.1
+
+- (use) Fix massive bug in cell keyboard nagivation, where pressing an arrow key would focus the next cell and select the next next cell
+- (a11y) Expand abbrevation (`strats` ⇒ `strategies`)
+- (css) Fix some incorrect rtl display
+
 ## v0.35.0
 
 - (use) Multiselectable cells! Select multiple cells at the same time with <kbd>Ctrl</kbd>+Click

Todo

@@ -19,6 +19,8 @@
 ## Code
 
 [ ] Migrate undo functionality completely to the solver
+[ ] Make rtl handling consistent in the css files
+[ ] rtl keyboard navigation should be inverted?
 
 ## Features
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "solver",
-  "version": "0.35.0",
+  "version": "0.35.1",
   "private": true,
   "homepage": "https://icecream17.github.io/solver",
   "dependencies": {

src/Api/Solver.test.tsx

@@ -10,7 +10,7 @@ jest.setTimeout(17000);
 
 beforeEach(() => {
    render(<App />);
-   switchTab("strats");
+   switchTab("strategies");
 })
 
 test("Import board", async () => {

src/App.test.tsx

@@ -19,7 +19,7 @@ test('a header exists', () => {
 })
 
 test("Strategy sections exist", () => {
-   switchTab('strats')
+   switchTab('strategies')
    expect(screen.getByRole('group', { name: 'strategies' })).toBeInTheDocument()
    expect(screen.getByRole('group', { name: 'controls' })).toBeInTheDocument()
 })

src/Elems/Aside.test.tsx

@@ -11,7 +11,7 @@ beforeEach(() => {
 })
 
 function getTogglers () {
-   switchTab('strats')
+   switchTab('strategies')
 
    try {
       return screen.getAllByRole('checkbox')

src/Elems/Aside.tsx

@@ -23,7 +23,7 @@ type AsideState = Readonly<{
 }>
 
 
-const tabNames = ["solving tools", "strats"]
+const tabNames = ["solving tools", "strategies"]
 
 /**
  * Currently a window of tabs

src/Elems/AsideElems/StrategyItem.css

@@ -34,7 +34,6 @@
    position: absolute;
    left: -1rem;
    margin: 0; /* Now sanitize.css works! */
-   vertical-align: middle;
    text-align: center;
    line-height: inherit;
 
@@ -55,7 +54,7 @@
 
    .StrategyToggler:dir(rtl) {
       left: initial;
-      right: -1rem;
+      right: 0;
    }
 }
 
@@ -72,7 +71,7 @@
 
    [dir=rtl] .StrategyToggler {
       left: initial;
-      right: -1rem;
+      right: 0;
    }
 }
 

src/Elems/AsideElems/StrategyList.css

@@ -9,12 +9,18 @@
    border-right: 5px solid rgb(183, 193, 194);
 }
 
-.StrategyItem.isCurrent[dir=rtl] {
-   border-left: 5px solid rgb(183, 193, 194);
-}
-
-.StrategyItem.isCurrent:dir(rtl) {
-   border-left: 5px solid rgb(183, 193, 194);
+@supports selector(:dir(rtl)) {
+   .StrategyItem.isCurrent:dir(rtl) {
+      border-right: initial;
+      border-left: 5px solid rgb(183, 193, 194);
+   }
+}
+
+@supports not selector(:dir(rtl)) {
+   .StrategyItem.isCurrent[dir=rtl] {
+      border-right: initial;
+      border-left: 5px solid rgb(183, 193, 194);
+   }
 }
 
 /* Alternating colors - note that the markers are not in color */

src/Elems/AsideElems/Tabs.test.tsx

@@ -22,7 +22,7 @@ beforeEach(() => {
 
 test('Tab: When focus moves into the tab list, focus on the active tab element', () => {
    const dummyTextarea = screen.getByTestId("dummy-focusable")
-   const stratsTab = screen.getByRole("tab", { name: "strats" })
+   const stratsTab = screen.getByRole("tab", { name: "strategies" })
    userEvent.click(stratsTab)
    dummyTextarea.focus()
    userEvent.tab()
@@ -39,7 +39,7 @@ test.skip('Tab: When in the tab list, focus on the tabpanel unless the first mea
 
 test.skip('Left / Right arrow keys', () => {
    const solveToolsTab = screen.getByRole("tab", { name: "solving tools" })
-   const stratsTab = screen.getByRole("tab", { name: "strats" })
+   const stratsTab = screen.getByRole("tab", { name: "strategies" })
 
    userEvent.click(stratsTab)
    userEvent.keyboard('{ArrowLeft}')
@@ -58,7 +58,7 @@ test.skip('Left / Right arrow keys', () => {
 
 test('Home / End', () => {
    const solveToolsTab = screen.getByRole("tab", { name: "solving tools" })
-   const stratsTab = screen.getByRole("tab", { name: "strats" })
+   const stratsTab = screen.getByRole("tab", { name: "strategies" })
 
    userEvent.click(stratsTab)
    userEvent.keyboard('{Home}')

src/Elems/MainElems/Sudoku.css

@@ -57,6 +57,7 @@ td.Cell:nth-child(3n):not(:last-child) {
 
 [dir=rtl] td.Cell:nth-child(3n):not(:last-child),
 td.Cell:nth-child(3n):not(:last-child):dir(rtl) {
+   border-right: initial;
    border-left: var(--large-border) solid var(--border-color);
 }
 

src/Elems/MainElems/Sudoku.test.tsx

@@ -47,19 +47,26 @@ test("Focused buttonCells are highlighted", () => {
    fireEvent.blur(buttonCell)
    expect(buttonCell).toHaveAttribute('data-active', 'false') // selection disabled
 
+   // An actual bug - pressing right moved the focus right and selected the cell right
+   const nextCell = getButtonCellElement(0, 2)
+   userEvent.click(buttonCell)
+   userEvent.keyboard('{ArrowRight}')
+   expect(nextCell).not.toHaveAttribute('data-active') // not selected
+   userEvent.click(buttonCell)
+
    // Multi-select
-   const nextCell = getButtonCellElement(3, 4)
+   const buttonCell2 = getButtonCellElement(3, 4)
    userEvent.keyboard('{Control>}')
-   userEvent.click(nextCell)
+   userEvent.click(buttonCell2)
    expect(buttonCell).toHaveAttribute('data-active', 'true') // selected
-   expect(nextCell).toHaveAttribute('data-active', 'true') // selected
-   fireEvent.blur(nextCell)
+   expect(buttonCell2).toHaveAttribute('data-active', 'true') // selected
+   fireEvent.blur(buttonCell2)
    expect(buttonCell).toHaveAttribute('data-active', 'false') // selection disabled
-   expect(nextCell).toHaveAttribute('data-active', 'false') // selection disabled
-   userEvent.click(nextCell)
+   expect(buttonCell2).toHaveAttribute('data-active', 'false') // selection disabled
+   userEvent.click(buttonCell2)
    userEvent.keyboard('{/Control}{Escape}')
    expect(buttonCell).not.toHaveAttribute('data-active') // not selected
-   expect(nextCell).not.toHaveAttribute('data-active') // not selected
+   expect(buttonCell2).not.toHaveAttribute('data-active') // not selected
 })
 
 test("Setting a cell to a digit", () => {
@@ -179,6 +186,10 @@ test("Cell keyboard navigation: Tab + Arrows", () => {
    tryKey('{ArrowDown}', 0, 7)
    tryKey('{ArrowRight}', 0, 8)
    tryKey('{ArrowRight}', 0, 0)
+   tryKey('{ArrowRight>}{ArrowDown}{/ArrowRight}', 1, 1) // Actual bug
+   tryKey('{ArrowRight>3/}', 1, 4) // Repeat
+   // tryKey('{ArrowUp>}{ArrowLeft>4/}{/ArrowUp}', 7, 1) // Diagonal repeat
+   // I tested this manually and it works so... idk
 })
 
 test("Cell keyboard navigation: End / Home", () => {

src/Elems/MainElems/Sudoku.tsx

@@ -170,6 +170,7 @@ export default class Sudoku extends React.Component<SudokuProps> {
    // The effect of Ctrl is to add (or remove) only one cell to the selection.
 
    whenCellFocus(cell: Cell, _event: React.FocusEvent) {
+      // console.debug("focus", cell.id)
       const ctrlMultiselect = keysPressed.has('Control') && !keysPressed.has('Tab')
       if (!ctrlMultiselect) {
          this.cellsSelected.clear()
@@ -181,6 +182,7 @@ export default class Sudoku extends React.Component<SudokuProps> {
    }
 
    whenCellBlur(cell: Cell, event: React.FocusEvent) {
+      // console.debug("blur", cell.id)
       // When <kbd>Escape</kbd> blurs a cell, the selection could be empty
       // in which case, do nothing
       if (this.selectionStatus === null) {
@@ -208,6 +210,7 @@ export default class Sudoku extends React.Component<SudokuProps> {
     * Edit: I'm not sure if I should actually follow that section
     */
    whenCellKeydown(targetCell: Cell, event: React.KeyboardEvent) {
+      // console.debug("keyboard", targetCell.id, event.repeat, event.key)
       // Use default behavior when tabbing
       if (event.key === 'Tab') {
          return
@@ -219,17 +222,24 @@ export default class Sudoku extends React.Component<SudokuProps> {
       // This has no effect on the listeners. SAFETY
       keysPressed.add(event.key)
 
+      // Let's say the user pressed both Up and Right at basically the same time
+      // What the code sees: Up, Right, UpEnd, RightEnd
+      // What the code does: Up, Up+Right
+      // We will only consider other keys to repeat when a repeat event occurs
+      const keysToProcess = event.repeat ? keysPressed : [event.key]
+
       const newSelected = new Set<CellID>()
 
       const target = event.target as HTMLDivElement
       const shiftHeld = keysPressed.has('Shift')
       const ctrlHeld = keysPressed.has('Control')
 
-      for (let {row, column} of this.cellsSelected) {
+      // Copy/save previous cells selected, since it changes during the loop
+      for (let {row, column} of [...this.cellsSelected]) {
          let cell = this.props.sudoku.cells[row][column]
          const wasTarget = cell === targetCell
 
-         for (const key of keysPressed) {
+         for (const key of keysToProcess) {
             if (cell == null) {
                break
             }

src/Elems/Version.tsx

@@ -9,7 +9,7 @@ import StaticComponent from './StaticComponent';
  * <Version />
  */
 function Version () {
-   return <span className="Version">v0.35.0</span>
+   return <span className="Version">v0.35.1</span>
 }
 
 export default StaticComponent(Version)