TINY-9283: Improved colorswatch ui functionality. (#8247)

* TINY-9283: Improved colorswatch ui functionality.

* TINY-9283: Reworking test to BDD style

* TINY-9283: Improved existing test, added new tests for previousSelector

* TINY-9283: Test title fixups.

* TINY-9283: Changelog updates-

Co-authored-by: Morgan Smith <morgan.smith@tiny.cloud>

modules/alloy/CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## Unreleased
 
 ### Added
+- Added `previousSelector` optional property to `MenuMatrixMovementSpec`, to start with the selected item in focus. #TINY-9283
 - Added the `onToggled` callback for the `FloatingToolbarButton` component. #TINY-9271
 - Added the `onOpened` and `onClosed` callbacks for the `SplitFloatingToolbar` component. #TINY-9271
 

modules/alloy/src/main/ts/ephox/alloy/ui/schema/MenuSchema.ts

@@ -1,5 +1,5 @@
 import { FieldSchema, StructureSchema } from '@ephox/boulder';
-import { Fun, Obj } from '@ephox/katamari';
+import { Fun, Obj, Optional } from '@ephox/katamari';
 
 import { Composing } from '../../api/behaviour/Composing';
 import { Highlighting } from '../../api/behaviour/Highlighting';
@@ -42,6 +42,7 @@ const configureMatrix = (detail: MenuDetail, movementInfo: MenuMatrixMovement):
     row: movementInfo.rowSelector,
     cell: '.' + detail.markers.item
   },
+  previousSelector: movementInfo.previousSelector,
   focusManager: detail.focusManager
 });
 
@@ -108,7 +109,8 @@ const schema = Fun.constant([
       ],
       matrix: [
         Fields.output('config', configureMatrix),
-        FieldSchema.required('rowSelector')
+        FieldSchema.required('rowSelector'),
+        FieldSchema.defaulted('previousSelector', Optional.none),
       ],
       menu: [
         FieldSchema.defaulted('moveOnTab', true),

modules/alloy/src/main/ts/ephox/alloy/ui/types/MenuTypes.ts

@@ -1,3 +1,6 @@
+import { Optional } from '@ephox/katamari';
+import { SugarElement } from '@ephox/sugar';
+
 import { AlloyBehaviourRecord } from '../../api/behaviour/Behaviour';
 import { AlloyComponent } from '../../api/component/ComponentApi';
 import { SketchBehaviours } from '../../api/component/SketchBehaviours';
@@ -19,6 +22,7 @@ export interface MenuGridMovementSpec {
 export interface MenuMatrixMovementSpec {
   mode: 'matrix';
   rowSelector: string;
+  previousSelector?: (comp: AlloyComponent) => Optional<SugarElement<HTMLElement>>;
 }
 
 export interface MenuNormalMovementSpec {
@@ -42,6 +46,7 @@ export interface MenuMatrixMovement {
   mode: 'matrix';
   config: (detail: MenuDetail, movementInfo: MenuMovement) => MatrixConfigSpec;
   rowSelector: string;
+  previousSelector: (comp: AlloyComponent) => Optional<SugarElement<HTMLElement>>;
 }
 
 export interface MenuNormalMovement {

modules/alloy/src/test/ts/browser/ui/dropdown/MatrixMenuTest.ts

@@ -1,10 +1,11 @@
-import { ApproxStructure, Assertions, Chain, Keyboard, Keys, NamedChain, StructAssert, UiFinder } from '@ephox/agar';
-import { UnitTest } from '@ephox/bedrock-client';
-import { Arr } from '@ephox/katamari';
+import { ApproxStructure, Assertions, Keyboard, Keys, TestStore, UiFinder } from '@ephox/agar';
+import { context, describe, it } from '@ephox/bedrock-client';
+import { Arr, Optional } from '@ephox/katamari';
 import { SugarElement } from '@ephox/sugar';
 
 import * as AddEventsBehaviour from 'ephox/alloy/api/behaviour/AddEventsBehaviour';
 import * as Behaviour from 'ephox/alloy/api/behaviour/Behaviour';
+import { AlloyComponent } from 'ephox/alloy/api/component/ComponentApi';
 import * as GuiFactory from 'ephox/alloy/api/component/GuiFactory';
 import { AlloySpec } from 'ephox/alloy/api/component/SpecTypes';
 import * as AlloyEvents from 'ephox/alloy/api/events/AlloyEvents';
@@ -14,126 +15,164 @@ import * as GuiSetup from 'ephox/alloy/api/testhelpers/GuiSetup';
 import { Menu } from 'ephox/alloy/api/ui/Menu';
 import * as MenuEvents from 'ephox/alloy/menu/util/MenuEvents';
 import * as TestDropdownMenu from 'ephox/alloy/test/dropdown/TestDropdownMenu';
-
-UnitTest.asynctest('MatrixMenuTest', (success, failure) => {
-
-  GuiSetup.setup((store, _doc, _body) => GuiFactory.build(
-    Menu.sketch({
-      value: 'test-menu-1',
-      items: Arr.map([
-        { type: 'item', data: { value: 'alpha', meta: { }}, hasSubmenu: false },
-        { type: 'item', data: { value: 'beta', meta: { }}, hasSubmenu: false },
-        { type: 'item', data: { value: 'gamma', meta: { }}, hasSubmenu: false },
-        { type: 'item', data: { value: 'delta', meta: { }}, hasSubmenu: false }
-      ], TestDropdownMenu.renderItem),
-      dom: {
-        tag: 'ol',
-        classes: [ 'test-menu' ]
-      },
-
-      movement: {
-        mode: 'matrix',
-
-        rowSelector: '.row-class'
-      },
-
-      components: [
-        Menu.parts.items({
-          preprocess: (items: AlloySpec[]) => {
-            const chunks = Arr.chunk(items, 2);
-            return Arr.map(chunks, (c) => ({
-              dom: {
-                tag: 'div',
-                classes: [ 'row-class' ]
-              },
-              components: c
-            }));
-          }
+import { MenuMovementSpec } from 'ephox/alloy/ui/types/MenuTypes';
+
+describe('browser.ui.dropdown.MatrixMenuTest', () => {
+  const selectedClass = 'selected-item';
+
+  const assertSelectedStates = (label: string, expectedPos: number, menu: SugarElement<HTMLElement>) =>
+    Assertions.assertStructure(
+      label,
+      ApproxStructure.build(
+        (s, _str, arr) => {
+          const classcheck = (index: number) =>
+            index === expectedPos ?
+              arr.has(selectedClass) :
+              arr.not(selectedClass);
+
+          return s.element('ol', {
+            classes: [
+              arr.has('test-menu')
+            ],
+            children: [
+              s.element('div', {
+                classes: [ arr.has('row-class') ],
+                children: [
+                  s.element('li', { classes: [ classcheck(0) ] }),
+                  s.element('li', { classes: [ classcheck(1) ] })
+                ]
+              }),
+              s.element('div', {
+                classes: [ arr.has('row-class') ],
+                children: [
+                  s.element('li', { classes: [ classcheck(2) ] }),
+                  s.element('li', { classes: [ classcheck(3) ] })
+                ]
+              })
+            ]
+          });
+        }
+      ), menu);
+
+  const makeGuiHook = (movementSpec: MenuMovementSpec) => {
+    return GuiSetup.bddSetup(
+      (store, _doc, _body) => GuiFactory.build(
+        Menu.sketch({
+          value: 'test-menu-1',
+          items: Arr.map([
+            { type: 'item', data: { value: 'alpha', meta: { }}, hasSubmenu: false },
+            { type: 'item', data: { value: 'beta', meta: { }}, hasSubmenu: false },
+            { type: 'item', data: { value: 'gamma', meta: { }}, hasSubmenu: false },
+            { type: 'item', data: { value: 'delta', meta: { }}, hasSubmenu: false }
+          ], TestDropdownMenu.renderItem),
+          dom: {
+            tag: 'ol',
+            classes: [ 'test-menu' ]
+          },
+
+          movement: movementSpec,
+
+          components: [
+            Menu.parts.items({
+              preprocess: (items: AlloySpec[]) => {
+                const chunks = Arr.chunk(items, 2);
+                return Arr.map(chunks, (c) => ({
+                  dom: {
+                    tag: 'div',
+                    classes: [ 'row-class' ]
+                  },
+                  components: c
+                }));
+              }
+            })
+          ],
+
+          markers: {
+            item: TestDropdownMenu.markers().item,
+            selectedItem: TestDropdownMenu.markers().selectedItem
+          },
+
+          menuBehaviours: Behaviour.derive([
+            AddEventsBehaviour.config('menu-test-behaviour', [
+              AlloyEvents.run(MenuEvents.focus(), store.adder('menu.events.focus'))
+            ])
+          ])
         })
-      ],
-
-      markers: {
-        item: TestDropdownMenu.markers().item,
-        selectedItem: TestDropdownMenu.markers().selectedItem
-      },
-
-      menuBehaviours: Behaviour.derive([
-        AddEventsBehaviour.config('menu-test-behaviour', [
-          AlloyEvents.run(MenuEvents.focus(), store.adder('menu.events.focus'))
-        ])
-      ])
-    })
-  ), (_doc, _body, _gui, component, store) => {
-    // TODO: Flesh out test.
-    const cAssertStructure = (label: string, expected: StructAssert) => Chain.op((element: SugarElement<HTMLOListElement>) => {
-      Assertions.assertStructure(label, expected, element);
-    });
-
-    const cTriggerFocusItem = Chain.op((target: SugarElement<HTMLLIElement>) => {
-      AlloyTriggers.dispatch(component, target, SystemEvents.focusItem());
+      )
+    );
+  };
+
+  const assertInitialFocusIsOnElement = (store: TestStore<string>, menuComponent: AlloyComponent, position: number, message: string) => {
+    AlloyTriggers.dispatch(menuComponent, menuComponent.element, SystemEvents.focus());
+    assertFocusIsOnItem(menuComponent, position, message);
+    store.assertEq('After focusItem event', [ 'menu.events.focus' ]);
+    store.clear();
+  };
+
+  const assertFocusIsOnItem = (menuComponent: AlloyComponent, position: number, message: string) =>
+    assertSelectedStates(message, position, menuComponent.element);
+
+  const setFocusOnItem = (store: TestStore<string>, menuComponent: AlloyComponent, element: SugarElement<Element>, position: number, message: string) => {
+    AlloyTriggers.dispatch(menuComponent, element, SystemEvents.focusItem());
+    // Focus item on alpha should select alpha (the first item)
+    assertFocusIsOnItem(menuComponent, position, message);
+    store.assertEq('After focusItem event', [ 'menu.events.focus' ]);
+    store.clear();
+  };
+
+  const keyboardNavigationMovement = (store: TestStore<string>, menuComponent: AlloyComponent, key: number, position: number, message: string) => {
+    Keyboard.keydown(key, { }, menuComponent.element);
+    assertFocusIsOnItem(menuComponent, position, message);
+    store.assertEq('After pressing key', [ 'menu.events.focus' ]);
+    store.clear();
+  };
+
+  context('No previous selector', () => {
+    const hook = makeGuiHook({
+      mode: 'matrix',
+      rowSelector: '.row-class'
     });
 
-    const cAssertSelectedStates = (label: string, expected: boolean[]) => NamedChain.direct('menu', cAssertStructure(label, ApproxStructure.build((s, _str, arr) => s.element('ol', {
-      classes: [
-        arr.has('test-menu')
-      ],
-      children: [
-        s.element('div', {
-          classes: [ arr.has('row-class') ],
-          children: [
-            s.element('li', { classes: [ (expected[0] ? arr.has : arr.not)('selected-item') ] }),
-            s.element('li', { classes: [ (expected[1] ? arr.has : arr.not)('selected-item') ] })
-          ]
-        }),
-        s.element('div', {
-          classes: [ arr.has('row-class') ],
-          children: [
-            s.element('li', { classes: [ (expected[2] ? arr.has : arr.not)('selected-item') ] }),
-            s.element('li', { classes: [ (expected[3] ? arr.has : arr.not)('selected-item') ] })
-          ]
-        })
-      ]
-    }))), '_');
-
-    return [
-      Chain.asStep({}, [
-        NamedChain.asChain([
-          NamedChain.writeValue('menu', component.element),
-          NamedChain.direct('menu', UiFinder.cFindIn('li[data-value="alpha"]'), 'alpha'),
-          NamedChain.direct('menu', UiFinder.cFindIn('li[data-value="beta"]'), 'beta'),
-
-          store.cAssertEq('Before focusItem event', [ ]),
+    it('TINY-9283: Basic Focusing', () => {
+      const menuComponent = hook.component();
+      const store = hook.store();
 
-          NamedChain.direct('alpha', cTriggerFocusItem, '_'),
+      // Find the alpha and beta list items
+      const alphaItem = UiFinder.findIn(menuComponent.element, 'li[data-value="alpha"]').getOrDie();
+      const betaItem = UiFinder.findIn(menuComponent.element, 'li[data-value="beta"]').getOrDie();
 
-          cAssertSelectedStates('After focusing item on alpha', [ true, false, false, false ]),
+      store.assertEq('Before focusItem event', [ ]);
+      assertInitialFocusIsOnElement(store, menuComponent, 0, 'Focus should be on alpha');
 
-          store.cAssertEq('After focusItem event (alpha)', [ 'menu.events.focus' ]),
-
-          store.cClear,
-          NamedChain.direct('beta', cTriggerFocusItem, '_'),
-          cAssertSelectedStates('After focusing item on beta', [ false, true, false, false ]),
-          store.cAssertEq('After focusItem event (beta)', [ 'menu.events.focus' ]),
-          store.cClear,
+      setFocusOnItem(store, menuComponent, alphaItem, 0, 'After focusing on alpha');
+      setFocusOnItem(store, menuComponent, betaItem, 1, 'After focusing on beta');
+    });
 
-          NamedChain.direct('menu', Chain.op((menu) => {
-            Keyboard.keydown(Keys.down(), { }, menu);
-          }), '_'),
+    it('TINY-9283: Keyboard navigation', () => {
+      const menuComponent = hook.component();
+      const store = hook.store();
 
-          cAssertSelectedStates('After pressing down on menu (with beta focus)', [ false, false, false, true ]),
-          store.cAssertEq('After pressing down on beta', [ 'menu.events.focus' ]),
-          store.cClear,
+      store.assertEq('Before focusItem event', [ ]);
+      assertInitialFocusIsOnElement(store, menuComponent, 0, 'Focus should be on alpha');
 
-          NamedChain.direct('menu', Chain.op((menu) => {
-            Keyboard.keydown(Keys.left(), { }, menu);
-          }), '_'),
+      keyboardNavigationMovement(store, menuComponent, Keys.right(), 1, 'Press right to go from alpha to beta ( column movement )');
+      keyboardNavigationMovement(store, menuComponent, Keys.down(), 3, 'Press down to move from beta to delta ( row movement )');
+      keyboardNavigationMovement(store, menuComponent, Keys.left(), 2, 'Press left to go from delta to gamma ( column movement )');
+      keyboardNavigationMovement(store, menuComponent, Keys.up(), 0, 'Press up to move from gamma to alpha ( row movement )');
+    });
+  });
 
-          cAssertSelectedStates('After pressing left on menu (with delta focus)', [ false, false, true, false ]),
-          store.cAssertEq('After pressing left on delta', [ 'menu.events.focus' ]),
-          store.cClear
+  context('Previous selector', () => {
+    const hook = makeGuiHook({
+      mode: 'matrix',
+      rowSelector: '.row-class',
+      previousSelector: (component) => Optional.some(UiFinder.findIn<HTMLElement>(component.element, 'li[data-value="beta"]').getOrDie())
+    });
 
-        ])
-      ])
-    ];
-  }, success, failure);
+    it('TINY-9283: Position starts as expected', () => {
+      const menuComponent = hook.component();
+      const store = hook.store();
+      assertInitialFocusIsOnElement(store, menuComponent, 1, 'Focus should be on beta');
+    });
+  });
 });

modules/oxide/src/less/theme/components/color-swatch/color-swatch.less

@@ -70,6 +70,27 @@
       background: @toolbar-button-hover-background-color;
     }
   }
+
+  div.tox-swatch:not(.tox-swatch--remove) {
+    svg {
+      display: none;
+      fill: @swatch-picker-button-icon-fill;
+      height: @swatch-picker-button-icon-height;
+      margin: calc((@swatch-size - @swatch-picker-button-icon-height) / 2) calc((@swatch-size - @swatch-picker-button-icon-width) / 2);
+      width: @swatch-picker-button-icon-width;
+
+      path {
+        fill: #fff;
+        paint-order: stroke;
+        stroke: #222f3e;
+        stroke-width: 2px;
+      }
+    }
+
+    &[aria-checked="true"] svg {
+      display: block;
+    }
+  }
 }
 
 .tox:not([dir=rtl]) {

modules/tinymce/CHANGELOG.md

@@ -19,10 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - Transparent elements, like anchors, are now allowed in the root of the editor body if they contain blocks. #TINY-9172
+- Colorswatch keyboard navigation now starts on currently selected color if present in the colorswatch. #TINY-9283
 - `setContent` is now allowed to accept any custom keys and values as a second options argument. #TINY-9143
 
 ### Improved
 - Transparent elements, like anchors, can now contain block elements. #TINY-9172
+- Colorswatch now displays a checkmark for selected color. #TINY-9283
 - Color picker dialog now starts on the appropriate color for the cursor position. #TINY-9213
 
 ### Fixed

modules/tinymce/src/themes/silver/main/ts/ui/core/color/Options.ts

@@ -18,6 +18,7 @@ const mapColors = (colorMap: string[]): Menu.ChoiceMenuItemSpec[] => {
     colors.push({
       text: colorMap[i + 1],
       value: '#' + Transformations.anyToHex(colorMap[i]).value,
+      icon: 'checkmark',
       type: 'choiceitem'
     });
   }