Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Accessibility): Color alone is used to convey information #336

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 10 additions & 5 deletions coral-base-button/src/scripts/BaseButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {

if (this.iconPosition === iconPosition.LEFT) {
this.appendChild(label);
} else {
}
else {
this.insertBefore(label, this.firstChild);
}
}
Expand Down Expand Up @@ -407,7 +408,8 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {

if (value === variant._CUSTOM) {
this.classList.remove(CLASSNAME);
} else {
}
else {
this.classList.add(...VARIANT_MAP[value]);

if (value === variant.ACTION || value === variant.QUIET_ACTION) {
Expand Down Expand Up @@ -505,7 +507,8 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
if (this._variant !== variant._CUSTOM) {
if (this._variant === variant.ACTION || this._variant === variant.QUIET_ACTION) {
label.classList.add(`${ACTION_CLASSNAME}-label`);
} else {
}
else {
label.classList.add(`${CLASSNAME}-label`);
}
}
Expand Down Expand Up @@ -612,7 +615,8 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
// Don't add duplicated icons
if (iconAdded) {
this.removeChild(child);
} else {
}
else {
// Conserve existing icon element to content
this._elements.icon = child;
fragment.appendChild(child);
Expand All @@ -622,7 +626,8 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
// Avoid content zone to be voracious
else if (contentZoneProvided) {
fragment.appendChild(child);
} else {
}
else {
// Move anything else into the label
label.appendChild(child);
}
Expand Down
51 changes: 34 additions & 17 deletions coral-base-component/src/scripts/BaseComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
// Try to get the method
if (typeof methodNameOrFunction === 'function') {
return methodNameOrFunction;
} else if (typeof methodNameOrFunction === 'string') {
}
else if (typeof methodNameOrFunction === 'string') {
if (!obj[methodNameOrFunction]) {
throw new Error(`Coral.Component: Unable to add ${eventName} listener for ${obj.toString()}, method
${methodNameOrFunction} not found`);
Expand All @@ -53,7 +54,8 @@
}

return listener;
} else if (methodNameOrFunction) {
}
else if (methodNameOrFunction) {
// If we're passed something that's truthy (like an object), but it's not a valid method name or a function, get
// angry
throw new Error(`Coral.Component: Unable to add ${eventName} listener for ${obj.toString()}, ${methodNameOrFunction}
Expand All @@ -67,7 +69,7 @@
Add local event and key combo listeners for this component, store global event/key combo listeners for later.
@ignore
*/
const delegateEvents = function () {

Check warning on line 72 in coral-base-component/src/scripts/BaseComponent.js

View workflow job for this annotation

GitHub Actions / build (12.x)

Function has a complexity of 19. Maximum allowed is 15
/*
Add listeners to new event
- Include in hash
Expand Down Expand Up @@ -147,7 +149,8 @@
selector: selector,
listener: listener
});
} else {
}
else {
this._globalEvents = this._globalEvents || [];
this._globalEvents.push({eventName, selector, listener, isCapture});
}
Expand All @@ -164,16 +167,19 @@

// Add listener locally
this._keys.on(eventName, selector, listener);
} else if (isResize) {
}
else if (isResize) {
if (selector) {
elements = document.querySelectorAll(selector);
for (let i = 0 ; i < elements.length ; ++i) {

Check warning on line 174 in coral-base-component/src/scripts/BaseComponent.js

View workflow job for this annotation

GitHub Actions / build (12.x)

Blocks are nested too deeply (5). Maximum allowed is 4
commons.addResizeListener(elements[i], listener);
}
} else {
}
else {
commons.addResizeListener(this, listener);
}
} else {
}
else {
this._vent.on(eventName, selector, listener, isCapture);
}
}
Expand Down Expand Up @@ -272,7 +278,8 @@

if (found) {
break;
} else {
}
else {
found = find(obj[key], constructorToFind);
}
}
Expand All @@ -291,7 +298,8 @@
if (constructor._namespace) {
constructorName.push(constructor._namespace.value);
constructor = constructor._namespace.parent;
} else {
}
else {
constructor = false;
}
}
Expand All @@ -315,7 +323,8 @@
// todo better check for coral-component
if(typeof child._ignoreConnectedCallback === 'boolean') {
child._ignoreConnectedCallback = value;
} else {
}
else {
_recursiveIgnoreConnectedCallback(child, value);
}
}
Expand Down Expand Up @@ -474,16 +483,19 @@
}
// Insert new node
insert.call(this, value);
} else if (oldNode && oldNode.parentNode) {
}
else if (oldNode && oldNode.parentNode) {
commons._log('warn', `${this._componentName} does not define an insert method for content zone ${handle}, falling back to replace.`);
// Old way -- assume we have an old node
this._elements[handle].parentNode.replaceChild(value, this._elements[handle]);
} else {
}
else {
commons._log('error', `${this._componentName} does not define an insert method for content zone ${handle}, falling back to append.`);
// Just append, which may introduce bugs, but at least doesn't crazy
this.appendChild(value);
}
} else {
}
else {
// we need to remove the content zone if it exists
oldNode = this._elements[handle];
if (oldNode && oldNode.parentNode) {
Expand All @@ -507,12 +519,14 @@
this._reflectedAttribute = true;
this.setAttribute(attributeName, '');
this._reflectedAttribute = false;
} else if (!value && this.hasAttribute(attributeName)) {
}
else if (!value && this.hasAttribute(attributeName)) {
this._reflectedAttribute = true;
this.removeAttribute(attributeName);
this._reflectedAttribute = false;
}
} else if (this.getAttribute(attributeName) !== String(value)) {
}
else if (this.getAttribute(attributeName) !== String(value)) {
this._reflectedAttribute = true;
this.setAttribute(attributeName, value);
this._reflectedAttribute = false;
Expand Down Expand Up @@ -683,7 +697,8 @@
const setProperty = (prop, val) => {
if (isContentZone(prop)) {
updateContentZone(prop, val);
} else {
}
else {
this._silenced = silent;
/** @ignore */
this[prop] = val;
Expand All @@ -697,7 +712,8 @@
value = valueOrSilent;

setProperty(property, value);
} else {
}
else {
properties = propertyOrProperties;
silent = valueOrSilent;

Expand Down Expand Up @@ -763,7 +779,8 @@
let handler = event.detail.handler;
if(typeof handler === 'function') {
handler(this);
} else {
}
else {
throw new Error("Messenger handler should be a function");
}
}
Expand Down
6 changes: 4 additions & 2 deletions coral-base-formfield/src/scripts/BaseFormField.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ const BaseFormField = (superClass) => class extends superClass {
if (element.matches(LABELLABLE_ELEMENTS_SELECTOR)) {
this._updateForAttributes(value, elementId);
}
} else {
}
else {
// since no labelledby value was set, we remove everything
element.removeAttribute('aria-labelledby');
}
Expand Down Expand Up @@ -361,7 +362,8 @@ const BaseFormField = (superClass) => class extends superClass {
if (forAttribute === elementId) {
labelElement.removeAttribute('for');
}
} else {
}
else {
// if we do not have to remove, it does not matter the current value of the label, we can set it in every
// case
labelElement.setAttribute('for', elementId);
Expand Down
3 changes: 2 additions & 1 deletion coral-base-labellable/src/scripts/BaseLabellable.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ const BaseLabellable = (superClass) => class extends superClass {
attributeChangedCallback(name, oldValue, value) {
if (name === 'aria-label' || name === 'aria-labelledby') {
this._toggleIconAriaHidden();
} else {
}
else {
super.attributeChangedCallback(name, oldValue, value);
}
}
Expand Down
6 changes: 4 additions & 2 deletions coral-base-list/src/scripts/BaseList.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ const BaseList = (superClass) => class extends superClass {

if (index < items.length - 1) {
items[index + 1].focus();
} else {
}
else {
items[0].focus();
}
}
Expand All @@ -182,7 +183,8 @@ const BaseList = (superClass) => class extends superClass {

if (index > 0) {
items[index - 1].focus();
} else {
}
else {
items[items.length - 1].focus();
}
}
Expand Down
3 changes: 2 additions & 1 deletion coral-base-list/src/scripts/BaseListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ const BaseListItem = (superClass) => class extends BaseLabellable(superClass) {
const el = this._getIconElement();
if (transform.string(value) === '') {
el.remove();
} else {
}
else {
this.insertBefore(el, this.firstChild);
}

Expand Down
39 changes: 26 additions & 13 deletions coral-base-overlay/src/scripts/BaseOverlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ function hideEverythingBut(instance) {
// It's already true, don't bother setting
continue;
}
} else {
}
else {
// Nothing is hidden by default, store that
child._previousAriaHidden = 'false';
}
Expand Down Expand Up @@ -244,7 +245,8 @@ function createDocumentTabCaptureEls() {
}
}
});
} else {
}
else {
if (document.body.firstElementChild !== topTabCaptureEl) {
// Make sure we stay at the very top
document.body.insertBefore(topTabCaptureEl, document.body.firstChild);
Expand Down Expand Up @@ -325,7 +327,8 @@ function hideOrRepositionBackdrop() {
if (!keepBackdrop) {
// Hide the backdrop
doBackdropHide();
} else {
}
else {
// Reposition the backdrop
doRepositionBackdrop();
}
Expand All @@ -335,7 +338,8 @@ function hideOrRepositionBackdrop() {
const top = OverlayManager.top();
if (!top || !(top.instance.trapFocus === trapFocus.ON && top.instance._requestedBackdrop)) {
hideDocumentTabCaptureEls();
} else if (top && top.instance.trapFocus === trapFocus.ON && top.instance._requestedBackdrop) {
}
else if (top && top.instance.trapFocus === trapFocus.ON && top.instance._requestedBackdrop) {
createDocumentTabCaptureEls();
}
}
Expand Down Expand Up @@ -432,7 +436,8 @@ const BaseOverlay = (superClass) => class extends superClass {
this._vent.on('keydown', this._handleRootKeypress);
this._vent.on('focus', '[coral-tabcapture]', this._handleTabCaptureFocus);

} else if (this._trapFocus === trapFocus.OFF) {
}
else if (this._trapFocus === trapFocus.OFF) {
// Remove elements
this._elements.topTabCapture && this._elements.topTabCapture.remove();
this._elements.intermediateTabCapture && this._elements.intermediateTabCapture.remove();
Expand Down Expand Up @@ -547,7 +552,8 @@ const BaseOverlay = (superClass) => class extends superClass {
// Doesn't matter when we set aria-hidden true (nothing being announced)
if (open) {
this.removeAttribute('aria-hidden');
} else {
}
else {
this.setAttribute('aria-hidden', !open);
}

Expand All @@ -568,7 +574,8 @@ const BaseOverlay = (superClass) => class extends superClass {
// element that had focus before opening the overlay
(document.activeElement === document.body ? null : document.activeElement);
}
} else {
}
else {
// Release zIndex
this._popOverlay();
}
Expand Down Expand Up @@ -622,11 +629,13 @@ const BaseOverlay = (superClass) => class extends superClass {
if (this._overlayAnimationTime) {
// Wait for animation to complete
commons.transitionEnd(this, openComplete);
} else {
}
else {
// Execute immediately
openComplete();
}
} else {
}
else {
// Fade out
this.classList.remove('is-open');

Expand Down Expand Up @@ -657,7 +666,8 @@ const BaseOverlay = (superClass) => class extends superClass {
if (this._overlayAnimationTime) {
// Wait for animation to complete
commons.transitionEnd(this, closeComplete);
} else {
}
else {
// Execute immediately
closeComplete();
}
Expand Down Expand Up @@ -816,9 +826,11 @@ const BaseOverlay = (superClass) => class extends superClass {
// ON handles the focusing per accessibility recommendations
if (this.focusOnShow === focusOnShow.ON) {
this._focusOn('first');
} else if (this.focusOnShow instanceof HTMLElement) {
}
else if (this.focusOnShow instanceof HTMLElement) {
this.focusOnShow.focus(preventScroll(this));
} else if (typeof this.focusOnShow === 'string' && this.focusOnShow !== focusOnShow.OFF) {
}
else if (typeof this.focusOnShow === 'string' && this.focusOnShow !== focusOnShow.OFF) {
// we need to add :not([coral-tabcapture]) to avoid selecting the tab captures
const selectedElement = this.querySelector(`${this.focusOnShow}:not([coral-tabcapture])`);

Expand Down Expand Up @@ -1039,7 +1051,8 @@ const BaseOverlay = (superClass) => class extends superClass {
// Show the backdrop again
this._showBackdrop();
}
} else {
}
else {
// If overlay is closed, make sure that it is hidden with `display: none`,
// but set `visibility: visible` to ensure that the overlay will be included in accessibility name or description
// of an element that references it using `aria-labelledby` or `aria-describedby`.
Expand Down
2 changes: 2 additions & 0 deletions coral-component-colorinput/src/scripts/ColorInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,8 @@ const ColorInput = Decorator(class extends BaseFormField(BaseComponent(HTMLEleme
this._elements.colorPreview.style.backgroundColor = currentColor ? currentColor.rgbaValue : '';
this.classList.toggle('_coral-ColorInput--novalue', isValueEmpty);

this._elements.colorPreviewLabel.textContent = currentColor ? this._color.value : '';

// Update preview in overlay
const preview = this._elements.overlay.querySelector('._coral-ColorInput-preview');
if (preview) {
Expand Down