From ce0630f32b2206813e5cfd2c7842b2faa5141591 Mon Sep 17 00:00:00 2001
From: Thijs Louisse <Thijs.Louisse@ing.com>
Date: Thu, 27 Feb 2020 11:07:12 +0100
Subject: [PATCH 1/2] fix: normalize subclasser apis

---
 packages/button/src/LionButton.js             |  8 ++--
 packages/choice-input/src/ChoiceInputMixin.js |  4 +-
 packages/field/src/FormControlMixin.js        | 38 +++++++++----------
 packages/fieldset/src/FormGroupMixin.js       |  2 +-
 packages/form/src/LionForm.js                 | 14 +++----
 .../src/LionInputDatepicker.js                | 22 +++++------
 .../test-helpers/DatepickerInputObject.js     |  2 +-
 packages/input-range/src/LionInputRange.js    | 12 +++---
 packages/select-rich/src/LionSelectInvoker.js | 13 +++++--
 packages/select-rich/src/LionSelectRich.js    | 16 ++++----
 .../test/lion-select-invoker.test.js          |  8 ++--
 packages/switch/src/LionSwitch.js             | 12 +++---
 12 files changed, 79 insertions(+), 72 deletions(-)

diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js
index 3fb3df61e..4c63cefc3 100644
--- a/packages/button/src/LionButton.js
+++ b/packages/button/src/LionButton.js
@@ -31,7 +31,7 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
   render() {
     return html`
       <div class="btn">
-        ${this._renderBefore()}
+        ${this._beforeTemplate()}
         ${browserDetection.isIE11
           ? html`
               <div id="${this._buttonId}"><slot></slot></div>
@@ -39,7 +39,7 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
           : html`
               <slot></slot>
             `}
-        ${this._renderAfter()}
+        ${this._afterTemplate()}
         <slot name="_button"></slot>
         <div class="click-area"></div>
       </div>
@@ -47,12 +47,12 @@ export class LionButton extends DisabledWithTabIndexMixin(SlotMixin(LitElement))
   }
 
   // eslint-disable-next-line class-methods-use-this
-  _renderBefore() {
+  _beforeTemplate() {
     return html``;
   }
 
   // eslint-disable-next-line class-methods-use-this
-  _renderAfter() {
+  _afterTemplate() {
     return html``;
   }
 
diff --git a/packages/choice-input/src/ChoiceInputMixin.js b/packages/choice-input/src/ChoiceInputMixin.js
index 7460d47cc..9ef59f1d7 100644
--- a/packages/choice-input/src/ChoiceInputMixin.js
+++ b/packages/choice-input/src/ChoiceInputMixin.js
@@ -109,7 +109,7 @@ export const ChoiceInputMixin = superclass =>
       return html`
         <slot name="input"></slot>
         <div class="choice-field__graphic-container">
-          ${this.choiceGraphicTemplate()}
+          ${this._choiceGraphicTemplate()}
         </div>
         <div class="choice-field__label">
           <slot name="label"></slot>
@@ -117,7 +117,7 @@ export const ChoiceInputMixin = superclass =>
       `;
     }
 
-    choiceGraphicTemplate() {
+    _choiceGraphicTemplate() {
       return nothing;
     }
 
diff --git a/packages/field/src/FormControlMixin.js b/packages/field/src/FormControlMixin.js
index 6cfd78a90..7c22c5f7f 100644
--- a/packages/field/src/FormControlMixin.js
+++ b/packages/field/src/FormControlMixin.js
@@ -276,28 +276,28 @@ export const FormControlMixin = dedupeMixin(
       render() {
         return html`
           <div class="form-field__group-one">
-            ${this.groupOneTemplate()}
+            ${this._groupOneTemplate()}
           </div>
           <div class="form-field__group-two">
-            ${this.groupTwoTemplate()}
+            ${this._groupTwoTemplate()}
           </div>
         `;
       }
 
-      groupOneTemplate() {
+      _groupOneTemplate() {
         return html`
-          ${this.labelTemplate()} ${this.helpTextTemplate()}
+          ${this._labelTemplate()} ${this._helpTextTemplate()}
         `;
       }
 
-      groupTwoTemplate() {
+      _groupTwoTemplate() {
         return html`
-          ${this.inputGroupTemplate()} ${this.feedbackTemplate()}
+          ${this._inputGroupTemplate()} ${this._feedbackTemplate()}
         `;
       }
 
       // eslint-disable-next-line class-methods-use-this
-      labelTemplate() {
+      _labelTemplate() {
         return html`
           <div class="form-field__label">
             <slot name="label"></slot>
@@ -306,7 +306,7 @@ export const FormControlMixin = dedupeMixin(
       }
 
       // eslint-disable-next-line class-methods-use-this
-      helpTextTemplate() {
+      _helpTextTemplate() {
         return html`
           <small class="form-field__help-text">
             <slot name="help-text"></slot>
@@ -314,21 +314,21 @@ export const FormControlMixin = dedupeMixin(
         `;
       }
 
-      inputGroupTemplate() {
+      _inputGroupTemplate() {
         return html`
           <div class="input-group">
-            ${this.inputGroupBeforeTemplate()}
+            ${this._inputGroupBeforeTemplate()}
             <div class="input-group__container">
-              ${this.inputGroupPrefixTemplate()} ${this.inputGroupInputTemplate()}
-              ${this.inputGroupSuffixTemplate()}
+              ${this._inputGroupPrefixTemplate()} ${this._inputGroupInputTemplate()}
+              ${this._inputGroupSuffixTemplate()}
             </div>
-            ${this.inputGroupAfterTemplate()}
+            ${this._inputGroupAfterTemplate()}
           </div>
         `;
       }
 
       // eslint-disable-next-line class-methods-use-this
-      inputGroupBeforeTemplate() {
+      _inputGroupBeforeTemplate() {
         return html`
           <div class="input-group__before">
             <slot name="before"></slot>
@@ -336,7 +336,7 @@ export const FormControlMixin = dedupeMixin(
         `;
       }
 
-      inputGroupPrefixTemplate() {
+      _inputGroupPrefixTemplate() {
         return !Array.from(this.children).find(child => child.slot === 'prefix')
           ? nothing
           : html`
@@ -347,7 +347,7 @@ export const FormControlMixin = dedupeMixin(
       }
 
       // eslint-disable-next-line class-methods-use-this
-      inputGroupInputTemplate() {
+      _inputGroupInputTemplate() {
         return html`
           <div class="input-group__input">
             <slot name="input"></slot>
@@ -355,7 +355,7 @@ export const FormControlMixin = dedupeMixin(
         `;
       }
 
-      inputGroupSuffixTemplate() {
+      _inputGroupSuffixTemplate() {
         return !Array.from(this.children).find(child => child.slot === 'suffix')
           ? nothing
           : html`
@@ -366,7 +366,7 @@ export const FormControlMixin = dedupeMixin(
       }
 
       // eslint-disable-next-line class-methods-use-this
-      inputGroupAfterTemplate() {
+      _inputGroupAfterTemplate() {
         return html`
           <div class="input-group__after">
             <slot name="after"></slot>
@@ -375,7 +375,7 @@ export const FormControlMixin = dedupeMixin(
       }
 
       // eslint-disable-next-line class-methods-use-this
-      feedbackTemplate() {
+      _feedbackTemplate() {
         return html`
           <div class="form-field__feedback">
             <slot name="feedback"></slot>
diff --git a/packages/fieldset/src/FormGroupMixin.js b/packages/fieldset/src/FormGroupMixin.js
index 7d967abd9..e39815392 100644
--- a/packages/fieldset/src/FormGroupMixin.js
+++ b/packages/fieldset/src/FormGroupMixin.js
@@ -194,7 +194,7 @@ export const FormGroupMixin = dedupeMixin(
       }
 
       // eslint-disable-next-line class-methods-use-this
-      inputGroupTemplate() {
+      _inputGroupTemplate() {
         return html`
           <div class="input-group">
             <slot></slot>
diff --git a/packages/form/src/LionForm.js b/packages/form/src/LionForm.js
index db02fa385..78d699fb5 100644
--- a/packages/form/src/LionForm.js
+++ b/packages/form/src/LionForm.js
@@ -22,16 +22,16 @@ export class LionForm extends LionFieldset {
     this.__teardownEventsForLionForm();
   }
 
-  get formElement() {
+  get _formNode() {
     return this.querySelector('form');
   }
 
   submit() {
-    this.formElement.submit();
+    this._formNode.submit();
   }
 
   reset() {
-    this.formElement.reset();
+    this._formNode.reset();
   }
 
   /**
@@ -46,7 +46,7 @@ export class LionForm extends LionFieldset {
       this.submitGroup();
       this.dispatchEvent(new Event('submit', { bubbles: true }));
     };
-    this.formElement.addEventListener('submit', this._submit);
+    this._formNode.addEventListener('submit', this._submit);
 
     this._reset = ev => {
       ev.preventDefault();
@@ -54,11 +54,11 @@ export class LionForm extends LionFieldset {
       this.resetGroup();
       this.dispatchEvent(new Event('reset', { bubbles: true }));
     };
-    this.formElement.addEventListener('reset', this._reset);
+    this._formNode.addEventListener('reset', this._reset);
   }
 
   __teardownEventsForLionForm() {
-    this.formElement.removeEventListener('submit', this._submit);
-    this.formElement.removeEventListener('rest', this._reset);
+    this._formNode.removeEventListener('submit', this._submit);
+    this._formNode.removeEventListener('rest', this._reset);
   }
 }
diff --git a/packages/input-datepicker/src/LionInputDatepicker.js b/packages/input-datepicker/src/LionInputDatepicker.js
index c2f2e965d..3b7653cae 100644
--- a/packages/input-datepicker/src/LionInputDatepicker.js
+++ b/packages/input-datepicker/src/LionInputDatepicker.js
@@ -136,11 +136,11 @@ export class LionInputDatepicker extends OverlayMixin(LionInputDate) {
     ];
   }
 
-  get _invokerElement() {
+  get _invokerNode() {
     return this.querySelector(`#${this.__invokerId}`);
   }
 
-  get _calendarElement() {
+  get _calendarNode() {
     return this._overlayCtrl.contentNode.querySelector('#calendar');
   }
 
@@ -173,8 +173,8 @@ export class LionInputDatepicker extends OverlayMixin(LionInputDate) {
   }
 
   __toggleInvokerDisabled() {
-    if (this._invokerElement) {
-      this._invokerElement.disabled = this.disabled || this.readOnly;
+    if (this._invokerNode) {
+      this._invokerNode.disabled = this.disabled || this.readOnly;
     }
   }
 
@@ -217,8 +217,8 @@ export class LionInputDatepicker extends OverlayMixin(LionInputDate) {
 
   render() {
     return html`
-      ${this.labelTemplate()} ${this.helpTextTemplate()} ${this.inputGroupTemplate()}
-      ${this.feedbackTemplate()} ${this._overlayTemplate()}
+      ${this._labelTemplate()} ${this._helpTextTemplate()} ${this._inputGroupTemplate()}
+      ${this._feedbackTemplate()} ${this._overlayTemplate()}
     `;
   }
 
@@ -276,7 +276,7 @@ export class LionInputDatepicker extends OverlayMixin(LionInputDate) {
     this._overlayCtrl.show();
     await Promise.all([
       this._overlayCtrl.contentNode.updateComplete,
-      this._calendarElement.updateComplete,
+      this._calendarNode.updateComplete,
     ]);
     this._onCalendarOverlayOpened();
   }
@@ -286,10 +286,10 @@ export class LionInputDatepicker extends OverlayMixin(LionInputDate) {
    */
   _onCalendarOverlayOpened() {
     if (this._focusCentralDateOnCalendarOpen) {
-      if (this._calendarElement.selectedDate) {
-        this._calendarElement.focusSelectedDate();
+      if (this._calendarNode.selectedDate) {
+        this._calendarNode.focusSelectedDate();
       } else {
-        this._calendarElement.focusCentralDate();
+        this._calendarNode.focusCentralDate();
       }
     }
   }
@@ -339,7 +339,7 @@ export class LionInputDatepicker extends OverlayMixin(LionInputDate) {
    * @override Configures OverlayMixin
    */
   get _overlayInvokerNode() {
-    return this._invokerElement;
+    return this._invokerNode;
   }
 
   /**
diff --git a/packages/input-datepicker/test-helpers/DatepickerInputObject.js b/packages/input-datepicker/test-helpers/DatepickerInputObject.js
index 64c2d8eee..81f3e3393 100644
--- a/packages/input-datepicker/test-helpers/DatepickerInputObject.js
+++ b/packages/input-datepicker/test-helpers/DatepickerInputObject.js
@@ -39,7 +39,7 @@ export class DatepickerInputObject {
    */
 
   get invokerEl() {
-    return this.el._invokerElement;
+    return this.el._invokerNode;
   }
 
   get overlayEl() {
diff --git a/packages/input-range/src/LionInputRange.js b/packages/input-range/src/LionInputRange.js
index e3c3bb158..0c021b2a5 100644
--- a/packages/input-range/src/LionInputRange.js
+++ b/packages/input-range/src/LionInputRange.js
@@ -83,24 +83,24 @@ export class LionInputRange extends LocalizeMixin(LionInput) {
     }
   }
 
-  inputGroupTemplate() {
+  _inputGroupTemplate() {
     return html`
       <div>
         <span class="input-range__value">${formatNumber(this.formattedValue)}</span>
         <span class="input-range__unit">${this.unit}</span>
       </div>
       <div class="input-group">
-        ${this.inputGroupBeforeTemplate()}
+        ${this._inputGroupBeforeTemplate()}
         <div class="input-group__container">
-          ${this.inputGroupPrefixTemplate()} ${this.inputGroupInputTemplate()}
-          ${this.inputGroupSuffixTemplate()}
+          ${this._inputGroupPrefixTemplate()} ${this._inputGroupInputTemplate()}
+          ${this._inputGroupSuffixTemplate()}
         </div>
-        ${this.inputGroupAfterTemplate()}
+        ${this._inputGroupAfterTemplate()}
       </div>
     `;
   }
 
-  inputGroupInputTemplate() {
+  _inputGroupInputTemplate() {
     return html`
       <div class="input-group__input">
         <slot name="input"></slot>
diff --git a/packages/select-rich/src/LionSelectInvoker.js b/packages/select-rich/src/LionSelectInvoker.js
index fcc01c144..dd30134e0 100644
--- a/packages/select-rich/src/LionSelectInvoker.js
+++ b/packages/select-rich/src/LionSelectInvoker.js
@@ -10,9 +10,16 @@ import { html } from '@lion/core';
 export class LionSelectInvoker extends LionButton {
   static get properties() {
     return {
+      /**
+       * @desc the option Element that is currently selected
+       */
       selectedElement: {
         type: Object,
       },
+      /**
+       * @desc When the connected LionSelectRich insteance is readOnly,
+       * this should be reflected in the invoker as well
+       */
       readOnly: {
         type: Boolean,
         reflect: true,
@@ -32,7 +39,7 @@ export class LionSelectInvoker extends LionButton {
     };
   }
 
-  get contentWrapper() {
+  get _contentWrapperNode() {
     return this.shadowRoot.getElementById('content-wrapper');
   }
 
@@ -53,7 +60,7 @@ export class LionSelectInvoker extends LionButton {
     return ``;
   }
 
-  _renderBefore() {
+  _beforeTemplate() {
     return html`
       <div id="content-wrapper">
         ${this._contentTemplate()}
@@ -62,7 +69,7 @@ export class LionSelectInvoker extends LionButton {
   }
 
   // eslint-disable-next-line class-methods-use-this
-  _renderAfter() {
+  _afterTemplate() {
     return html`
       <slot name="after"></slot>
     `;
diff --git a/packages/select-rich/src/LionSelectRich.js b/packages/select-rich/src/LionSelectRich.js
index 22dbdda67..e6812ae22 100644
--- a/packages/select-rich/src/LionSelectRich.js
+++ b/packages/select-rich/src/LionSelectRich.js
@@ -157,8 +157,8 @@ export class LionSelectRich extends ChoiceGroupMixin(
     return this.formElements.findIndex(el => el.active === true);
   }
 
-  get scrollTarget() {
-    return this._overlayContentNode.scrollTarget || this._overlayContentNode;
+  get _scrollTargetNode() {
+    return this._overlayContentNode._scrollTargetNode || this._overlayContentNode;
   }
 
   set activeIndex(index) {
@@ -166,7 +166,7 @@ export class LionSelectRich extends ChoiceGroupMixin(
       const el = this.formElements[index];
       el.active = true;
 
-      if (!isInView(this.scrollTarget, el)) {
+      if (!isInView(this._scrollTargetNode, el)) {
         el.scrollIntoView({ behavior: 'smooth', block: 'nearest' });
       }
     }
@@ -239,8 +239,8 @@ export class LionSelectRich extends ChoiceGroupMixin(
 
   render() {
     return html`
-      ${this.labelTemplate()} ${this.helpTextTemplate()} ${this.inputGroupTemplate()}
-      ${this.feedbackTemplate()}
+      ${this._labelTemplate()} ${this._helpTextTemplate()} ${this._inputGroupTemplate()}
+      ${this._feedbackTemplate()}
       <slot name="_overlay-shadow-outlet"></slot>
     `;
   }
@@ -288,7 +288,7 @@ export class LionSelectRich extends ChoiceGroupMixin(
    * @override
    */
   // eslint-disable-next-line
-  inputGroupInputTemplate() {
+  _inputGroupInputTemplate() {
     return html`
       <div class="input-group__input">
         <slot name="invoker"></slot>
@@ -614,13 +614,13 @@ export class LionSelectRich extends ChoiceGroupMixin(
     this._overlayCtrl.addEventListener('hide', this.__overlayOnHide);
 
     this.__preventScrollingWithArrowKeys = this.__preventScrollingWithArrowKeys.bind(this);
-    this.scrollTarget.addEventListener('keydown', this.__preventScrollingWithArrowKeys);
+    this._scrollTargetNode.addEventListener('keydown', this.__preventScrollingWithArrowKeys);
   }
 
   __teardownOverlay() {
     this._overlayCtrl.removeEventListener('show', this.__overlayOnShow);
     this._overlayCtrl.removeEventListener('hide', this.__overlayOnHide);
-    this.scrollTarget.removeEventListener('keydown', this.__overlayOnHide);
+    this._scrollTargetNode.removeEventListener('keydown', this.__overlayOnHide);
   }
 
   __preventScrollingWithArrowKeys(ev) {
diff --git a/packages/select-rich/test/lion-select-invoker.test.js b/packages/select-rich/test/lion-select-invoker.test.js
index fdec095ac..526470252 100644
--- a/packages/select-rich/test/lion-select-invoker.test.js
+++ b/packages/select-rich/test/lion-select-invoker.test.js
@@ -19,7 +19,7 @@ describe('lion-select-invoker', () => {
     el.selectedElement = await fixture(`<div class="option"><h2>I am</h2><p>2 lines</p></div>`);
     await el.updateComplete;
 
-    expect(el.contentWrapper).lightDom.to.equal(
+    expect(el._contentWrapperNode).lightDom.to.equal(
       `
         <h2>I am</h2>
         <p>2 lines</p>
@@ -37,7 +37,7 @@ describe('lion-select-invoker', () => {
     el.selectedElement = await fixture(`<div class="option">just textContent</div>`);
     await el.updateComplete;
 
-    expect(el.contentWrapper).lightDom.to.equal('just textContent');
+    expect(el._contentWrapperNode).lightDom.to.equal('just textContent');
   });
 
   it('has tabindex="0"', async () => {
@@ -66,11 +66,11 @@ describe('lion-select-invoker', () => {
 
       el.selectedElement = await fixture(`<div class="option">cat</div>`);
       await el.updateComplete;
-      expect(el.contentWrapper).lightDom.to.equal('cat selected');
+      expect(el._contentWrapperNode).lightDom.to.equal('cat selected');
 
       el.selectedElement = await fixture(`<div class="option">dog</div>`);
       await el.updateComplete;
-      expect(el.contentWrapper).lightDom.to.equal('no valid selection');
+      expect(el._contentWrapperNode).lightDom.to.equal('no valid selection');
     });
   });
 });
diff --git a/packages/switch/src/LionSwitch.js b/packages/switch/src/LionSwitch.js
index e24be04d8..2155fe75b 100644
--- a/packages/switch/src/LionSwitch.js
+++ b/packages/switch/src/LionSwitch.js
@@ -30,23 +30,23 @@ export class LionSwitch extends ChoiceInputMixin(LionField) {
   render() {
     return html`
       <div class="form-field__group-one">
-        ${this.groupOneTemplate()}
+        ${this._groupOneTemplate()}
       </div>
       <div class="form-field__group-two">
-        ${this.groupTwoTemplate()}
+        ${this._groupTwoTemplate()}
       </div>
     `;
   }
 
-  groupOneTemplate() {
+  _groupOneTemplate() {
     return html`
-      ${this.labelTemplate()} ${this.helpTextTemplate()} ${this.feedbackTemplate()}
+      ${this._labelTemplate()} ${this._helpTextTemplate()} ${this._feedbackTemplate()}
     `;
   }
 
-  groupTwoTemplate() {
+  _groupTwoTemplate() {
     return html`
-      ${this.inputGroupTemplate()}
+      ${this._inputGroupTemplate()}
     `;
   }
 

From 1310bd491e5363ec86159c1ae390964b1fb9cc16 Mon Sep 17 00:00:00 2001
From: Thijs Louisse <Thijs.Louisse@ing.com>
Date: Thu, 27 Feb 2020 11:27:25 +0100
Subject: [PATCH 2/2] fix(form): leave accessible role to native form

---
 packages/form/src/LionForm.js        |  8 +++-----
 packages/form/test/lion-form.test.js | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/packages/form/src/LionForm.js b/packages/form/src/LionForm.js
index 78d699fb5..257716533 100644
--- a/packages/form/src/LionForm.js
+++ b/packages/form/src/LionForm.js
@@ -13,6 +13,9 @@ export class LionForm extends LionFieldset {
       super.connectedCallback();
     }
     this.__registerEventsForLionForm();
+
+    // @override LionFieldset: makes sure a11y is handled by ._formNode
+    this.removeAttribute('role');
   }
 
   disconnectedCallback() {
@@ -34,11 +37,6 @@ export class LionForm extends LionFieldset {
     this._formNode.reset();
   }
 
-  /**
-   * As we use a native form there is no need for a role
-   */
-  _setRole() {} // eslint-disable-line class-methods-use-this
-
   __registerEventsForLionForm() {
     this._submit = ev => {
       ev.preventDefault();
diff --git a/packages/form/test/lion-form.test.js b/packages/form/test/lion-form.test.js
index 7d47652eb..4fab34101 100644
--- a/packages/form/test/lion-form.test.js
+++ b/packages/form/test/lion-form.test.js
@@ -9,8 +9,10 @@ import {
 } from '@open-wc/testing';
 import { spy } from 'sinon';
 import { LionField } from '@lion/field';
+import { LionFieldset } from '@lion/fieldset';
 import '@lion/field/lion-field.js';
 import '@lion/fieldset/lion-fieldset.js';
+
 import '../lion-form.js';
 
 const childTagString = defineCE(
@@ -27,6 +29,26 @@ const formTagString = 'lion-form';
 const formTag = unsafeStatic(formTagString);
 
 describe('<lion-form>', () => {
+  it('is an instance of LionFieldSet', async () => {
+    const el = await fixture(html`
+      <${formTag}>
+        <form>
+        </form>
+      </${formTag}>
+    `);
+    expect(el).to.be.instanceOf(LionFieldset);
+  });
+
+  it('relies on the native form for its accessible role', async () => {
+    const el = await fixture(html`
+      <${formTag}>
+        <form>
+        </form>
+      </${formTag}>
+    `);
+    expect(el.getAttribute('role')).to.be.null;
+  });
+
   it('has a custom reset that gets triggered by native reset', async () => {
     const withDefaults = await fixture(html`
       <${formTag}>