Skip to content

Commit

Permalink
Revert "feat(color-picker): add accessibility (#448)"
Browse files Browse the repository at this point in the history
This reverts commit ced3b20.
  • Loading branch information
Nantawat-Poothong authored Nov 2, 2022
1 parent ced3b20 commit 6378be8
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 210 deletions.
1 change: 0 additions & 1 deletion documents/src/navigation.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type: nav
- [Calendar](./elements/calendar)
- [Checkbox](./elements/checkbox)
- [Color Dialog](./elements/color-dialog)
- [Color Picker](./elements/color-picker)
- [Combo Box](./elements/combo-box)
- [Counter](./elements/counter)
- [Datetime Picker](./elements/datetime-picker)
Expand Down
22 changes: 12 additions & 10 deletions documents/src/pages/elements/color-picker.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@ layout: default
::color-picker::
```
```css
section {
display:flex;
justify-content: left;
align-items: baseline;
height: 380px;
padding: 4px;
}
ef-color-picker {
margin-right: 2px;
}
<style>
section {
display:flex;
justify-content: left;
align-items: baseline;
height: 380px;
padding: 4px;
}
ef-color-picker {
margin-right: 2px;
}
</style>
```
```html
<section>
Expand Down
8 changes: 0 additions & 8 deletions documents/src/templates/import-element/ef-color-picker.md

This file was deleted.

4 changes: 1 addition & 3 deletions packages/elements/src/color-picker/__demo__/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,21 @@
Default Hex Value (Optional): <input id="hexInputDefault" class="hex-input" value="">
</div>
<div>
<p> Output: <span id="readbleColor"></span></p>
<p> Output: </p>
<textarea readonly id="defaultColorOutput"></textarea>
</div>
<script>
// Default Color Picker
const defaultColorPicker = document.getElementById('defaultColorPicker');
const hexInputDefault = document.getElementById('hexInputDefault');
const defaultColorOutput = document.getElementById('defaultColorOutput');
const readbleColor = document.getElementById('readbleColor');
hexInputDefault.addEventListener('change', event => {
defaultColorPicker.setAttribute('value', hexInputDefault.value);
});
defaultColorPicker.addEventListener('value-changed', event => {
const value = event.detail.value;
hexInputDefault.value = value;
defaultColorOutput.value += 'Value Change : ' + value + '\n';
setTimeout(() => readbleColor.textContent = value ? defaultColorPicker._colorAriaLabel : '', 0);
});
</script>
</demo-block>
Expand Down
15 changes: 1 addition & 14 deletions packages/elements/src/color-picker/__snapshots__/ColorPicker.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@
#### `DOM structure is correct`

```html
<div
aria-label=""
aria-live="polite"
part="aria-selection"
role="status"
>
</div>
<div
part="color-item"
style="background-color:#001EFF;"
Expand All @@ -23,13 +16,6 @@
#### `DOM structure is correct when opened`

```html
<div
aria-label=""
aria-live="polite"
part="aria-selection"
role="status"
>
</div>
<div part="color-item">
</div>
<ef-overlay-viewport>
Expand All @@ -39,6 +25,7 @@
draggable=""
offset="4"
opened=""
part="dialog"
role="dialog"
tabindex="-1"
with-shadow=""
Expand Down
26 changes: 7 additions & 19 deletions packages/elements/src/color-picker/__test__/color-picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ import { fixture, expect, elementUpdated, keyboardEvent, oneEvent } from '@refin
import '@refinitiv-ui/elements/color-picker';
import '@refinitiv-ui/elemental-theme/light/ef-color-picker';

/**
* Get private dialog element property
*/
export const getDialogEl = colorPicker => colorPicker.dialogRef.value;

describe('color-picker/ColorPicker', () => {

describe('DOM structure', () => {
Expand All @@ -30,13 +25,6 @@ describe('color-picker/ColorPicker', () => {
const el = await fixture('<ef-color-picker value="#001EFF"></ef-color-picker>');
expect(el.value).to.equal('#001EFF');
});
it('Should update aria-label when change value color', async () => {
const el = await fixture('<ef-color-picker value="#0000FF"></ef-color-picker>');
expect(el._colorAriaLabel).to.equal('blue');
el.value = '#ff0000';
await elementUpdated(el);
expect(el._colorAriaLabel).to.equal('red');
});
it('Should reset to default value when value is invalid', async () => {
const el = await fixture('<ef-color-picker value="hello"></ef-color-picker>');
expect(el.value).to.equal('');
Expand All @@ -55,7 +43,7 @@ describe('color-picker/ColorPicker', () => {
});
it('Should fires value-changed event when value change by user interactions', async () => {
const el = await fixture('<ef-color-picker value="#001EFF" opened></ef-color-picker>');
const dialogEl = getDialogEl(el);
const dialogEl = el.dialogEl;
const redInput = dialogEl.shadowRoot.getElementById('redInput');
const confirmBtn = dialogEl.shadowRoot.getElementById('confirmButton');
redInput.value = 200;
Expand All @@ -73,13 +61,13 @@ describe('color-picker/ColorPicker', () => {
const el = await fixture('<ef-color-picker></ef-color-picker>');
el.opened = true;
await elementUpdated(el);
expect(getDialogEl(el).hasAttribute('allow-nocolor')).to.be.equal(false);
expect(el.dialogEl.hasAttribute('allow-nocolor')).to.be.equal(false);
});
it('Should pass allow-nocolor property to color dialog', async () => {
const el = await fixture('<ef-color-picker allow-nocolor></ef-color-picker>');
el.opened = true;
await elementUpdated(el);
expect(getDialogEl(el).hasAttribute('allow-nocolor')).to.be.equal(true);
expect(el.dialogEl.hasAttribute('allow-nocolor')).to.be.equal(true);
});
});

Expand All @@ -88,13 +76,13 @@ describe('color-picker/ColorPicker', () => {
const el = await fixture('<ef-color-picker></ef-color-picker>');
el.click();
await elementUpdated(el);
expect(getDialogEl(el).opened).to.be.equal(true, 'clicking on color picker should open color dialog');
expect(el.dialogEl.opened).to.be.equal(true, 'clicking on color picker should open color dialog');
});
it('Should open dialog when opened programmatically', async () => {
const el = await fixture('<ef-color-picker></ef-color-picker>');
el.opened = true;
await elementUpdated(el);
expect(getDialogEl(el).hasAttribute('opened')).to.be.equal(true);
expect(el.dialogEl.hasAttribute('opened')).to.be.equal(true);
});
it('Should not open color dialog when disabled', async () => {
const el = await fixture('<ef-color-picker disabled></ef-color-picker>');
Expand All @@ -113,13 +101,13 @@ describe('color-picker/ColorPicker', () => {
const el = await fixture('<ef-color-picker></ef-color-picker>');
el.dispatchEvent(keyboardEvent('keydown', { key: 'Enter' }));
await elementUpdated(el);
expect(getDialogEl(el).opened).to.be.equal(true, 'Enter should open dialog');
expect(el.dialogEl.opened).to.be.equal(true, 'Enter should open dialog');
});
it('Should open dialog when press spacebar key', async () => {
const el = await fixture('<ef-color-picker></ef-color-picker>');
el.dispatchEvent(keyboardEvent('keydown', { key: 'Spacebar' }));
await elementUpdated(el);
expect(getDialogEl(el).opened).to.be.equal(true, 'Spacebar should open dialog');
expect(el.dialogEl.opened).to.be.equal(true, 'Spacebar should open dialog');
});
});
});
Expand Down
91 changes: 6 additions & 85 deletions packages/elements/src/color-picker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,14 @@ import {
import { customElement } from '@refinitiv-ui/core/decorators/custom-element.js';
import type { OpenedChangedEvent, ValueChangedEvent } from '../events';
import { property } from '@refinitiv-ui/core/decorators/property.js';
import { query } from '@refinitiv-ui/core/decorators/query.js';
import { styleMap } from '@refinitiv-ui/core/directives/style-map.js';
import { ifDefined } from '@refinitiv-ui/core/directives/if-defined.js';
import { VERSION } from '../version.js';
import { isHex, readableColor } from '@refinitiv-ui/utils/color.js';
import { ref, createRef, Ref } from '@refinitiv-ui/core/directives/ref.js';
import { isHex } from '@refinitiv-ui/utils/color.js';
import '../color-dialog/index.js';
import type { ColorDialog } from '../color-dialog/index.js';
import '@refinitiv-ui/phrasebook/locale/en/color-picker.js';
import { state } from '@refinitiv-ui/core/decorators/state.js';

import {
translate,
TranslatePromise,
TranslatePropertyKey
} from '@refinitiv-ui/translate';


const DIALOG_POSITION = ['right-start', 'right-end', 'right-middle', 'left-start', 'left-end', 'left-middle'];

Expand All @@ -53,14 +45,6 @@ export class ColorPicker extends ControlElement {
return VERSION;
}

protected readonly defaultRole: string | null = 'button';

/**
* Color Description for aria-label
*/
@state()
private _colorAriaLabel = '';

/**
* Set the color dialog to activate no-color option
*/
Expand All @@ -74,12 +58,6 @@ export class ColorPicker extends ControlElement {
@property({ type: String })
public lang = '';

/**
* Color picker internal translation strings
*/
@translate({ mode: 'promise', scope: 'ef-color-picker' })
protected colorTPromise!: TranslatePromise;

/**
* A `CSSResult` that will be used
* to style the host, slotted children
Expand All @@ -100,11 +78,6 @@ export class ColorPicker extends ControlElement {
`;
}

/**
* Reference to the color dialog
*/
private dialogRef: Ref<ColorDialog> = createRef();

private lazyRendered = false; /* speed up rendering by not populating color dialog on first load */

/**
Expand All @@ -113,6 +86,8 @@ export class ColorPicker extends ControlElement {
@property({ type: Boolean, reflect: true })
public opened = false;

@query('[part=dialog]') private dialogEl?: ColorDialog | null;

/**
* Check if value is valid HEX value (including #)
* @param value Value to check
Expand All @@ -138,16 +113,6 @@ export class ColorPicker extends ControlElement {
return !(this.disabled || this.readonly);
}

/**
* Called when connected to DOM
* @returns {void}
*/
public connectedCallback (): void {
super.connectedCallback();
// Indicating that this color picker has a dialog
this.setAttribute('aria-haspopup', 'dialog');
}

/**
* Called after the component is first rendered
* @param changedProperties Properties which have changed
Expand All @@ -157,7 +122,6 @@ export class ColorPicker extends ControlElement {
super.firstUpdated(changedProperties);
this.addEventListener('tap', this.onTap);
this.addEventListener('keydown', this.onKeyDown);
void this.updateColorAriaLabel();
}

/**
Expand All @@ -166,9 +130,6 @@ export class ColorPicker extends ControlElement {
* @returns {void}
*/
protected update (changedProperties: PropertyValues): void {
if (changedProperties.has(TranslatePropertyKey) || changedProperties.has('value')) {
void this.updateColorAriaLabel();
}
if (changedProperties.has('opened') && this.opened) {
this.lazyRendered = true;
}
Expand All @@ -187,7 +148,7 @@ export class ColorPicker extends ControlElement {
*/
private onTap (event: TapEvent): void {
const path = event.composedPath();
if ((this.dialogRef.value && path.includes(this.dialogRef.value)) || event.defaultPrevented) {
if ((this.dialogEl && path.includes(this.dialogEl)) || event.defaultPrevented) {
return; /* dialog is managed separately */
}
this.setOpened(!this.opened);
Expand Down Expand Up @@ -250,53 +211,14 @@ export class ColorPicker extends ControlElement {
this.setOpened(event.detail.value);
}

/**
* A template used to notify currently selected value for screen readers
* @returns template result
*/
private get selectionTemplate (): TemplateResult | null {
if (!this.value) {
return null;
}
return html`<div
part="aria-selection"
role="status"
aria-live="polite"
aria-label="${this._colorAriaLabel}"></div>`;
}

/**
* Helper method, used to set the color description.
* @returns {void}
*/
private async updateColorAriaLabel (): Promise<void> {
const { name, tone, main, percent, mixed } = readableColor(this.value);
if (name) {
this._colorAriaLabel = name;
return;
}
const translate = await Promise.all([
this.colorTPromise(main),
this.colorTPromise('WITH', { number: percent }),
this.colorTPromise(mixed)]);
const [mainT, percentT, mixedT] = translate;
const toneT = tone ? `${await this.colorTPromise(tone)}. ` : '';
if (percent) {
this._colorAriaLabel = `${toneT}${mainT} ${percentT} ${mixedT}`;
}
else {
this._colorAriaLabel = `${toneT}${mainT}`;
}
}

/**
* Color dialog template
*/
private get dialogTemplate (): TemplateResult | undefined {
if (this.lazyRendered) {
return html`<ef-color-dialog
offset="4"
${ref(this.dialogRef)}
part="dialog"
.lang=${ifDefined(this.lang || undefined)}
.value=${this.value}
.focusBoundary=${this}
Expand Down Expand Up @@ -324,7 +246,6 @@ export class ColorPicker extends ControlElement {
*/
protected render (): TemplateResult {
return html`
${this.selectionTemplate}
${this.colorItemTemplate}
${this.dialogTemplate}
`;
Expand Down
15 changes: 1 addition & 14 deletions packages/phrasebook/src/locale/de/color-picker.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
// Component docs https://ui.refinitiv.com/elements/color-picker
// Component docs https://elf.int.refinitiv.com/elements/color-picker.html
import { Phrasebook } from '../../translation.js';
import './shared.js';
import './color-dialog.js';

const translations = {
VERY_LIGHT: 'Very light',
LIGHT: 'Light',
VERY_DARK: 'Very dark',
DARK: 'Dark',
BLACK: 'Black',
WHITE: 'White',
WITH: 'With {number}%',
YELLOW: 'Yellow',
GREEN: 'Green',
CYAN: 'Cyan',
BLUE: 'Blue',
MAGENTA: 'Magenta',
RED: 'Red'
};

Phrasebook.define('de', 'ef-color-picker', translations);
Expand Down
Loading

0 comments on commit 6378be8

Please sign in to comment.