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

refactor: ensure all buttons have label and accessibility text #891

Merged
merged 1 commit into from Mar 18, 2024
Merged
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
12 changes: 6 additions & 6 deletions src/components/Sidebar.test.tsx
Expand Up @@ -61,7 +61,7 @@ describe('components/Sidebar.tsx', () => {
});

it('should refresh the notifications', () => {
const { getByLabelText } = render(
const { getByTitle } = render(
<AppContext.Provider
value={{ isLoggedIn: true, notifications: [], fetchNotifications }}
>
Expand All @@ -71,19 +71,19 @@ describe('components/Sidebar.tsx', () => {
</AppContext.Provider>,
);
fetchNotifications.mockReset();
fireEvent.click(getByLabelText('Refresh Notifications'));
fireEvent.click(getByTitle('Refresh Notifications'));
expect(fetchNotifications).toHaveBeenCalledTimes(1);
});

it('go to the settings route', () => {
const { getByLabelText } = render(
const { getByTitle } = render(
<AppContext.Provider value={{ isLoggedIn: true, notifications: [] }}>
<MemoryRouter>
<Sidebar />
</MemoryRouter>
</AppContext.Provider>,
);
fireEvent.click(getByLabelText('Settings'));
fireEvent.click(getByTitle('Settings'));
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/settings');
});

Expand All @@ -108,14 +108,14 @@ describe('components/Sidebar.tsx', () => {
});

it('should quit the app', () => {
const { getByLabelText } = render(
const { getByTitle } = render(
<AppContext.Provider value={{ isLoggedIn: false, notifications: [] }}>
<MemoryRouter>
<Sidebar />
</MemoryRouter>
</AppContext.Provider>,
);
fireEvent.click(getByLabelText('Quit App'));
fireEvent.click(getByTitle('Quit Gitify'));
expect(ipcRenderer.send).toHaveBeenCalledTimes(1);
expect(ipcRenderer.send).toHaveBeenCalledWith('app-quit');
});
Expand Down
14 changes: 9 additions & 5 deletions src/components/Sidebar.tsx
Expand Up @@ -66,36 +66,40 @@ export const Sidebar: React.FC = () => {
<>
<button
className={footerButtonClasses}
title="Refresh Notifications"
onClick={() => {
navigate('/', { replace: true });
fetchNotifications();
}}
aria-label="Refresh Notifications"
>
<IconRefresh className="w-3.5 h-3.5" />
<IconRefresh
className="w-3.5 h-3.5"
aria-label="Refresh Notifications"
/>
</button>

<button
className={footerButtonClasses}
title="Settings"
onClick={() => {
if (location.pathname.startsWith('/settings')) {
navigate('/', { replace: true });
} else {
navigate('/settings');
}
}}
aria-label="Settings"
>
<IconCog className="w-4 h-4" />
<IconCog className="w-4 h-4" aria-label="Settings" />
</button>
</>
)}

{!isLoggedIn && (
<button
className={footerButtonClasses}
title="Quit Gitify"
aria-label="Quit Gitify"
onClick={quitApp}
aria-label="Quit App"
>
<IconQuit className="w-3.5 h-3.5" />
</button>
Expand Down
6 changes: 4 additions & 2 deletions src/components/__snapshots__/Sidebar.test.tsx.snap
Expand Up @@ -86,9 +86,10 @@ exports[`components/Sidebar.tsx should render itself & its children (logged in)
className="py-4 px-3"
>
<button
aria-label="Quit App"
aria-label="Quit Gitify"
className="flex justify-evenly items-center bg-transparent border-0 w-full text-sm text-white my-1 py-2 cursor-pointer hover:text-gray-500 focus:outline-none"
onClick={[Function]}
title="Quit Gitify"
>
<svg
aria-hidden="true"
Expand Down Expand Up @@ -206,9 +207,10 @@ exports[`components/Sidebar.tsx should render itself & its children (logged out)
className="py-4 px-3"
>
<button
aria-label="Quit App"
aria-label="Quit Gitify"
className="flex justify-evenly items-center bg-transparent border-0 w-full text-sm text-white my-1 py-2 cursor-pointer hover:text-gray-500 focus:outline-none"
onClick={[Function]}
title="Quit Gitify"
>
<svg
aria-hidden="true"
Expand Down
9 changes: 6 additions & 3 deletions src/routes/Login.tsx
Expand Up @@ -38,24 +38,27 @@ export const LoginRoute: React.FC = () => {

<button
className={loginButtonClass}
onClick={loginUser}
title="Login with GitHub"
aria-label="Login with GitHub"
onClick={loginUser}
>
Login to GitHub
</button>

<button
className={loginButtonClass}
onClick={() => navigate('/login-enterprise')}
title="Login with GitHub Enterprise"
aria-label="Login with GitHub Enterprise"
onClick={() => navigate('/login-enterprise')}
>
Login to GitHub Enterprise
</button>

<button
className="bg-none hover:text-gray-800 dark:text-gray-100 dark:hover:text-gray-300 mt-4 focus:outline-none"
onClick={() => navigate('/login-token')}
title="Login with Personal Token"
aria-label="Login with Personal Token"
onClick={() => navigate('/login-token')}
>
<small>or login with a personal token</small>
</button>
Expand Down
10 changes: 7 additions & 3 deletions src/routes/LoginEnterprise.tsx
Expand Up @@ -85,9 +85,9 @@ export const LoginEnterpriseRoute: React.FC = () => {

<button
className="float-right px-4 py-2 my-4 bg-gray-300 font-semibold rounded text-sm text-center hover:bg-gray-500 hover:text-white dark:text-black focus:outline-none"
title="Login Button"
disabled={submitting || pristine}
type="submit"
title="Login Button"
>
Login
</button>
Expand All @@ -108,10 +108,14 @@ export const LoginEnterpriseRoute: React.FC = () => {
<div className="flex justify-between items-center mt-4 py-2 mx-8">
<button
className="focus:outline-none"
aria-label="Go Back"
title="Go Back"
onClick={() => navigate(-1)}
>
<ArrowLeftIcon size={20} className="hover:text-gray-400" />
<ArrowLeftIcon
size={20}
className="hover:text-gray-400"
aria-label="Go Back"
/>
</button>

<h3 className="text-lg font-semibold">Login with GitHub Enterprise</h3>
Expand Down
10 changes: 7 additions & 3 deletions src/routes/LoginWithToken.tsx
Expand Up @@ -95,9 +95,9 @@ export const LoginWithToken: React.FC = () => {

<button
className="float-right px-4 py-2 my-4 bg-gray-300 font-semibold rounded text-sm text-center hover:bg-gray-500 hover:text-white dark:text-black focus:outline-none"
title="Submit Button"
disabled={submitting || pristine}
type="submit"
title="Submit Button"
>
Submit
</button>
Expand All @@ -120,10 +120,14 @@ export const LoginWithToken: React.FC = () => {
<div className="flex justify-between items-center mt-4 py-2 mx-8">
<button
className="focus:outline-none"
aria-label="Go Back"
title="Go Back"
onClick={() => navigate(-1)}
>
<ArrowLeftIcon size={20} className="hover:text-gray-400" />
<ArrowLeftIcon
size={20}
className="hover:text-gray-400"
aria-label="Go Back"
/>
</button>

<h3 className="text-lg font-semibold">Login with an access token</h3>
Expand Down
24 changes: 12 additions & 12 deletions src/routes/Settings.test.tsx
Expand Up @@ -46,10 +46,10 @@ describe('routes/Settings.tsx', () => {

it('should press the logout', async () => {
const logoutMock = jest.fn();
let getByLabelText;
let getByTitle;

await act(async () => {
const { getByLabelText: getByLabelTextLocal } = render(
const { getByTitle: getByLabelTextLocal } = render(
<AppContext.Provider
value={{
settings: mockSettings,
Expand All @@ -63,10 +63,10 @@ describe('routes/Settings.tsx', () => {
</AppContext.Provider>,
);

getByLabelText = getByLabelTextLocal;
getByTitle = getByLabelTextLocal;
});

fireEvent.click(getByLabelText('Logout'));
fireEvent.click(getByTitle('Logout'));

expect(logoutMock).toHaveBeenCalledTimes(1);

Expand Down Expand Up @@ -265,10 +265,10 @@ describe('routes/Settings.tsx', () => {
});

it('should go to the enterprise login route', async () => {
let getByLabelText;
let getByTitle;

await act(async () => {
const { getByLabelText: getByLabelTextLocal } = render(
const { getByTitle: getByTitleLocal } = render(
<AppContext.Provider
value={{
settings: mockSettings,
Expand All @@ -280,20 +280,20 @@ describe('routes/Settings.tsx', () => {
</MemoryRouter>
</AppContext.Provider>,
);
getByLabelText = getByLabelTextLocal;
getByTitle = getByTitleLocal;
});

fireEvent.click(getByLabelText('Login with GitHub Enterprise'));
fireEvent.click(getByTitle('Login with GitHub Enterprise'));
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-enterprise', {
replace: true,
});
});

it('should quit the app', async () => {
let getByLabelText;
let getByTitle;

await act(async () => {
const { getByLabelText: getByLabelTextLocal } = render(
const { getByTitle: getByTitleLocal } = render(
<AppContext.Provider
value={{ settings: mockSettings, accounts: mockAccounts }}
>
Expand All @@ -302,10 +302,10 @@ describe('routes/Settings.tsx', () => {
</MemoryRouter>
</AppContext.Provider>,
);
getByLabelText = getByLabelTextLocal;
getByTitle = getByTitleLocal;
});

fireEvent.click(getByLabelText('Quit Gitify'));
fireEvent.click(getByTitle('Quit Gitify'));
expect(ipcRenderer.send).toHaveBeenCalledWith('app-quit');
});

Expand Down
23 changes: 15 additions & 8 deletions src/routes/Settings.tsx
Expand Up @@ -84,10 +84,14 @@ export const SettingsRoute: React.FC = () => {
<div className="flex justify-between items-center mt-4 py-2 mx-8">
<button
className="focus:outline-none"
aria-label="Go Back"
title="Go Back"
onClick={() => navigate(-1)}
>
<ArrowLeftIcon size={20} className="hover:text-gray-400" />
<ArrowLeftIcon
size={20}
className="hover:text-gray-400"
aria-label="Go Back"
/>
</button>

<h3 className="text-lg font-semibold">Settings</h3>
Expand Down Expand Up @@ -165,26 +169,29 @@ export const SettingsRoute: React.FC = () => {
<div>
<button
className={footerButtonClass}
aria-label="Login with GitHub Enterprise"
title="Login with GitHub Enterprise"
onClick={goToEnterprise}
>
<IconAddAccount className="w-5 h-5" />
<IconAddAccount
className="w-5 h-5"
aria-label="Login with GitHub Enterprise"
/>
</button>

<button
className={footerButtonClass}
aria-label="Logout"
title="Logout"
onClick={logoutUser}
>
<IconLogOut className="w-5 h-5" />
<IconLogOut className="w-5 h-5" aria-label="Logout" />
</button>

<button
className={`${footerButtonClass} mr-0`}
aria-label="Quit Gitify"
title="Quit Gitify"
onClick={quitApp}
>
<IconQuit className="w-5 h-5" />
<IconQuit className="w-5 h-5" aria-label="Quit Gitify" />
</button>
</div>
</div>
Expand Down
3 changes: 3 additions & 0 deletions src/routes/__snapshots__/Login.test.tsx.snap
Expand Up @@ -59,20 +59,23 @@ exports[`routes/Login.tsx should render itself & its children 1`] = `
aria-label="Login with GitHub"
className="w-48 py-2 my-2 bg-gray-300 font-semibold rounded text-xs text-center dark:text-black hover:bg-gray-500 hover:text-white focus:outline-none"
onClick={[Function]}
title="Login with GitHub"
>
Login to GitHub
</button>
<button
aria-label="Login with GitHub Enterprise"
className="w-48 py-2 my-2 bg-gray-300 font-semibold rounded text-xs text-center dark:text-black hover:bg-gray-500 hover:text-white focus:outline-none"
onClick={[Function]}
title="Login with GitHub Enterprise"
>
Login to GitHub Enterprise
</button>
<button
aria-label="Login with Personal Token"
className="bg-none hover:text-gray-800 dark:text-gray-100 dark:hover:text-gray-300 mt-4 focus:outline-none"
onClick={[Function]}
title="Login with Personal Token"
>
<small>
or login with a personal token
Expand Down
5 changes: 3 additions & 2 deletions src/routes/__snapshots__/LoginEnterprise.test.tsx.snap
Expand Up @@ -8,12 +8,13 @@ exports[`routes/LoginEnterprise.js renders correctly 1`] = `
className="flex justify-between items-center mt-4 py-2 mx-8"
>
<button
aria-label="Go Back"
className="focus:outline-none"
onClick={[Function]}
title="Go Back"
>
<svg
aria-hidden="true"
aria-hidden="false"
aria-label="Go Back"
className="hover:text-gray-400"
fill="currentColor"
focusable="false"
Expand Down
5 changes: 3 additions & 2 deletions src/routes/__snapshots__/LoginWithToken.test.tsx.snap
Expand Up @@ -8,12 +8,13 @@ exports[`routes/LoginWithToken.js renders correctly 1`] = `
className="flex justify-between items-center mt-4 py-2 mx-8"
>
<button
aria-label="Go Back"
className="focus:outline-none"
onClick={[Function]}
title="Go Back"
>
<svg
aria-hidden="true"
aria-hidden="false"
aria-label="Go Back"
className="hover:text-gray-400"
fill="currentColor"
focusable="false"
Expand Down