Commit c0fed047 authored by samdbeckham's avatar samdbeckham

Adds changes from @pburdette review

- Falls back to the close selector
- Renames all CLOSE variables to DISMISS for clarity
- Prettifies the files
parent a66aea57
...@@ -5,12 +5,14 @@ ...@@ -5,12 +5,14 @@
export default function initAlertHandler() { export default function initAlertHandler() {
const DISMISSIBLE_SELECTORS = ['.gl-alert', '.gl-banner']; const DISMISSIBLE_SELECTORS = ['.gl-alert', '.gl-banner'];
const CLOSE_SELECTOR = '[aria-label="Dismiss"]'; const DISMISS_LABEL = '[aria-label="Dismiss"]';
const DISMISS_CLASS = '.gl-alert-dismiss';
DISMISSIBLE_SELECTORS.forEach(selector => { DISMISSIBLE_SELECTORS.forEach(selector => {
const elements = document.querySelectorAll(selector); const elements = document.querySelectorAll(selector);
elements.forEach(element => elements.forEach(element => {
element.querySelector(CLOSE_SELECTOR).addEventListener('click', () => element.remove()), const button = element.querySelector(DISMISS_LABEL) || element.querySelector(DISMISS_CLASS);
); button.addEventListener('click', () => element.remove());
});
}); });
} }
...@@ -4,17 +4,19 @@ import initAlertHandler from '~/alert_handler'; ...@@ -4,17 +4,19 @@ import initAlertHandler from '~/alert_handler';
describe('Alert Handler', () => { describe('Alert Handler', () => {
const ALERT_CLASS = 'gl-alert'; const ALERT_CLASS = 'gl-alert';
const BANNER_CLASS = 'gl-banner'; const BANNER_CLASS = 'gl-banner';
const CLOSE_LABEL = 'Dismiss'; const DISMISS_CLASS = 'gl-alert-dismiss';
const DISMISS_LABEL = 'Dismiss';
const generateHtml = parentClass => const generateHtml = parentClass =>
`<div class="${parentClass}"> `<div class="${parentClass}">
<button aria-label="${CLOSE_LABEL}">Dismiss</button> <button aria-label="${DISMISS_LABEL}">Dismiss</button>
</div>`; </div>`;
const findFirstAlert = () => document.querySelector(`.${ALERT_CLASS}`); const findFirstAlert = () => document.querySelector(`.${ALERT_CLASS}`);
const findFirstBanner = () => document.querySelector(`.${BANNER_CLASS}`); const findFirstBanner = () => document.querySelector(`.${BANNER_CLASS}`);
const findAllAlerts = () => document.querySelectorAll(`.${ALERT_CLASS}`); const findAllAlerts = () => document.querySelectorAll(`.${ALERT_CLASS}`);
const findFirstCloseButton = () => document.querySelector(`[aria-label="${CLOSE_LABEL}"]`); const findFirstDismissButton = () => document.querySelector(`[aria-label="${DISMISS_LABEL}"]`);
const findFirstDismissButtonByClass = () => document.querySelector(`.${DISMISS_CLASS}`);
describe('initAlertHandler', () => { describe('initAlertHandler', () => {
describe('with one alert', () => { describe('with one alert', () => {
...@@ -28,7 +30,7 @@ describe('Alert Handler', () => { ...@@ -28,7 +30,7 @@ describe('Alert Handler', () => {
}); });
it('should dismiss the alert on click', () => { it('should dismiss the alert on click', () => {
findFirstCloseButton().click(); findFirstDismissButton().click();
expect(findFirstAlert()).not.toExist(); expect(findFirstAlert()).not.toExist();
}); });
}); });
...@@ -44,7 +46,7 @@ describe('Alert Handler', () => { ...@@ -44,7 +46,7 @@ describe('Alert Handler', () => {
}); });
it('should dismiss only one alert on click', () => { it('should dismiss only one alert on click', () => {
findFirstCloseButton().click(); findFirstDismissButton().click();
expect(findAllAlerts()).toHaveLength(1); expect(findAllAlerts()).toHaveLength(1);
}); });
}); });
...@@ -60,9 +62,30 @@ describe('Alert Handler', () => { ...@@ -60,9 +62,30 @@ describe('Alert Handler', () => {
}); });
it('should dismiss the banner on click', () => { it('should dismiss the banner on click', () => {
findFirstCloseButton().click(); findFirstDismissButton().click();
expect(findFirstBanner()).not.toExist(); expect(findFirstBanner()).not.toExist();
}); });
}); });
// Dismiss buttons *should* have the correct aria labels, but some of them won't
// because legacy code isn't always a11y compliant.
// This tests that the fallback for the incorrectly labelled buttons works.
describe('with a mislabelled dismiss button', () => {
beforeEach(() => {
setHTMLFixture(`<div class="${ALERT_CLASS}">
<button class="${DISMISS_CLASS}">Dismiss</button>
</div>`);
initAlertHandler();
});
it('should render the banner', () => {
expect(findFirstAlert()).toExist();
});
it('should dismiss the banner on click', () => {
findFirstDismissButtonByClass().click();
expect(findFirstAlert()).not.toExist();
});
});
}); });
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment