Skip to content

Commit 92471bf

Browse files
setchysetchy
authored
refactor(auth): update ux and terminology (#1122)
1 parent 638c0b8 commit 92471bf

16 files changed

+565
-483
lines changed

‎src/app.tsx‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import { Loading } from './components/Loading';
1111
import { Sidebar } from './components/Sidebar';
1212
import { AppContext, AppProvider } from './context/App';
1313
import { LoginRoute } from './routes/Login';
14-
import { LoginEnterpriseRoute } from './routes/LoginEnterprise';
15-
import { LoginWithToken } from './routes/LoginWithToken';
14+
import { LoginWithOAuthApp } from './routes/LoginWithOAuthApp';
15+
import { LoginWithPersonalAccessToken } from './routes/LoginWithPersonalAccessToken';
1616
import { NotificationsRoute } from './routes/Notifications';
1717
import { SettingsRoute } from './routes/Settings';
1818

@@ -51,12 +51,13 @@ export const App = () => {
5151
</RequireAuth>
5252
}
5353
/>
54+
5455
<Route path="/login" element={<LoginRoute />} />
5556
<Route
56-
path="/login-enterprise"
57-
element={<LoginEnterpriseRoute />}
57+
path="/login-personal-access-token"
58+
element={<LoginWithPersonalAccessToken />}
5859
/>
59-
<Route path="/login-token" element={<LoginWithToken />} />
60+
<Route path="/login-oauth-app" element={<LoginWithOAuthApp />} />
6061
</Routes>
6162
</div>
6263
</Router>

‎src/components/fields/Button.tsx‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,36 @@ import { openExternalLink } from '../../utils/comms';
55
export interface IButton {
66
name: string;
77
label: string;
8+
class?: string;
89
icon?: Icon;
9-
url?: string;
1010
size?: number;
11-
class?: string;
11+
url?: string;
12+
onClick?: () => void;
1213
disabled?: boolean;
1314
type?: 'button' | 'submit';
1415
}
1516

1617
export const Button: FC<IButton> = (props: IButton) => {
1718
const baseClass =
18-
'rounded bg-gray-300 font-semibold rounded text-sm text-center hover:bg-gray-500 hover:text-white dark:text-black focus:outline-none cursor-pointer';
19+
'rounded bg-gray-300 font-semibold text-xs text-center hover:bg-gray-500 hover:text-white dark:text-black focus:outline-none cursor-pointer px-2 py-1';
1920
return (
2021
<button
2122
type={props.type ?? 'button'}
2223
title={props.label}
2324
aria-label={props.label}
2425
className={`${props.class} ${baseClass}`}
2526
disabled={props.disabled}
26-
onClick={() => props.url && openExternalLink(props.url)}
27+
onClick={() => {
28+
if (props.url) {
29+
return openExternalLink(props.url);
30+
}
31+
32+
if (props.onClick) {
33+
return props.onClick();
34+
}
35+
}}
2736
>
28-
{props.icon && <props.icon className="mr-1" size={props.size && 14} />}
37+
{props.icon && <props.icon className="mr-1" size={props.size ?? 14} />}
2938
{props.name}
3039
</button>
3140
);

‎src/components/fields/__snapshots__/Button.test.tsx.snap‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/routes/Login.test.tsx‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,30 @@ describe('routes/Login.tsx', () => {
4949
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/', { replace: true });
5050
});
5151

52-
it('should navigate to login with github enterprise', () => {
52+
it('should navigate to login with personal access token', () => {
5353
render(
5454
<MemoryRouter>
5555
<LoginRoute />
5656
</MemoryRouter>,
5757
);
5858

59-
fireEvent.click(screen.getByLabelText('Login with GitHub Enterprise'));
59+
fireEvent.click(screen.getByLabelText('Login with Personal Access Token'));
6060

61-
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-enterprise');
61+
expect(mockNavigate).toHaveBeenNthCalledWith(
62+
1,
63+
'/login-personal-access-token',
64+
);
65+
});
66+
67+
it('should navigate to login with oauth app', () => {
68+
render(
69+
<MemoryRouter>
70+
<LoginRoute />
71+
</MemoryRouter>,
72+
);
73+
74+
fireEvent.click(screen.getByLabelText('Login with OAuth App'));
75+
76+
expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app');
6277
});
6378
});

‎src/routes/Login.tsx‎

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ const { ipcRenderer } = require('electron');
33
import { type FC, useContext, useEffect } from 'react';
44
import { useNavigate } from 'react-router-dom';
55

6+
import { KeyIcon, PersonIcon } from '@primer/octicons-react';
67
import { Logo } from '../components/Logo';
8+
import { Button } from '../components/fields/Button';
79
import { AppContext } from '../context/App';
810

911
export const LoginRoute: FC = () => {
@@ -26,9 +28,6 @@ export const LoginRoute: FC = () => {
2628
}
2729
}, []); */
2830

29-
const loginButtonClass =
30-
'w-50 px-2 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';
31-
3231
return (
3332
<div className="flex flex-1 flex-col justify-center items-center p-4 bg-white dark:bg-gray-dark dark:text-white">
3433
<Logo className="w-16 h-16" isDark />
@@ -37,38 +36,34 @@ export const LoginRoute: FC = () => {
3736
GitHub Notifications <br /> on your menu bar.
3837
</div>
3938

39+
<div className="font-semibold text-center text-sm italic">Login with</div>
4040
{
4141
// FIXME: Temporarily disable Login with GitHub (OAuth) as it's currently broken and requires a rewrite - see #485 #561 #747
42-
/*
43-
<button
44-
className={loginButtonClass}
45-
title="Login with GitHub"
46-
aria-label="Login with GitHub"
42+
/*
43+
<Button
44+
name="GitHub"
45+
icon={MarkGitHubIcon}
46+
label="Login with GitHub"
47+
class="w-50 px-2 py-2 my-2 text-xs"
4748
onClick={loginUser}
48-
>
49-
Login to GitHub
50-
</button> */
49+
/>
50+
*/
5151
}
5252

53-
<button
54-
type="button"
55-
className={loginButtonClass}
56-
title="Login with Personal Token"
57-
aria-label="Login with Personal Token"
58-
onClick={() => navigate('/login-token')}
59-
>
60-
Login with Personal Access Token
61-
</button>
62-
63-
<button
64-
type="button"
65-
className={loginButtonClass}
66-
title="Login with GitHub Enterprise"
67-
aria-label="Login with GitHub Enterprise"
68-
onClick={() => navigate('/login-enterprise')}
69-
>
70-
Login to GitHub Enterprise
71-
</button>
53+
<Button
54+
name="Personal Access Token"
55+
icon={KeyIcon}
56+
label="Login with Personal Access Token"
57+
class="py-2 mt-2"
58+
onClick={() => navigate('/login-personal-access-token')}
59+
/>
60+
<Button
61+
name="OAuth App"
62+
icon={PersonIcon}
63+
label="Login with OAuth App"
64+
class="py-2 mt-2"
65+
onClick={() => navigate('/login-oauth-app')}
66+
/>
7267
</div>
7368
);
7469
};

src/routes/LoginEnterprise.test.tsx renamed to src/routes/LoginWithOAuthApp.test.tsx

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ import { shell } from 'electron';
66
import { mockedEnterpriseAccounts } from '../__mocks__/mockedData';
77
import { AppContext } from '../context/App';
88
import type { AuthState } from '../types';
9-
import { LoginEnterpriseRoute, validate } from './LoginEnterprise';
9+
import { LoginWithOAuthApp, validate } from './LoginWithOAuthApp';
1010

1111
const mockNavigate = jest.fn();
1212
jest.mock('react-router-dom', () => ({
1313
...jest.requireActual('react-router-dom'),
1414
useNavigate: () => mockNavigate,
1515
}));
1616

17-
describe('routes/LoginEnterprise.tsx', () => {
17+
describe('routes/LoginWithOAuthApp.tsx', () => {
1818
const openExternalMock = jest.spyOn(shell, 'openExternal');
1919

2020
const mockAccounts: AuthState = {
@@ -33,7 +33,7 @@ describe('routes/LoginEnterprise.tsx', () => {
3333
const tree = TestRenderer.create(
3434
<AppContext.Provider value={{ accounts: mockAccounts }}>
3535
<MemoryRouter>
36-
<LoginEnterpriseRoute />
36+
<LoginWithOAuthApp />
3737
</MemoryRouter>
3838
</AppContext.Provider>,
3939
);
@@ -45,7 +45,7 @@ describe('routes/LoginEnterprise.tsx', () => {
4545
render(
4646
<AppContext.Provider value={{ accounts: mockAccounts }}>
4747
<MemoryRouter>
48-
<LoginEnterpriseRoute />
48+
<LoginWithOAuthApp />
4949
</MemoryRouter>
5050
</AppContext.Provider>,
5151
);
@@ -84,7 +84,7 @@ describe('routes/LoginEnterprise.tsx', () => {
8484
render(
8585
<AppContext.Provider value={{ accounts: mockAccounts }}>
8686
<MemoryRouter>
87-
<LoginEnterpriseRoute />
87+
<LoginWithOAuthApp />
8888
</MemoryRouter>
8989
</AppContext.Provider>,
9090
);
@@ -98,7 +98,7 @@ describe('routes/LoginEnterprise.tsx', () => {
9898
render(
9999
<AppContext.Provider value={{ accounts: mockAccounts }}>
100100
<MemoryRouter>
101-
<LoginEnterpriseRoute />
101+
<LoginWithOAuthApp />
102102
</MemoryRouter>
103103
</AppContext.Provider>,
104104
);
@@ -117,7 +117,7 @@ describe('routes/LoginEnterprise.tsx', () => {
117117
const { rerender } = render(
118118
<AppContext.Provider value={{ accounts: mockAccounts }}>
119119
<MemoryRouter>
120-
<LoginEnterpriseRoute />
120+
<LoginWithOAuthApp />
121121
</MemoryRouter>
122122
</AppContext.Provider>,
123123
);
@@ -132,7 +132,7 @@ describe('routes/LoginEnterprise.tsx', () => {
132132
}}
133133
>
134134
<MemoryRouter>
135-
<LoginEnterpriseRoute />
135+
<LoginWithOAuthApp />
136136
</MemoryRouter>
137137
</AppContext.Provider>,
138138
);
@@ -146,7 +146,7 @@ describe('routes/LoginEnterprise.tsx', () => {
146146
render(
147147
<AppContext.Provider value={{ accounts: mockAccounts }}>
148148
<MemoryRouter>
149-
<LoginEnterpriseRoute />
149+
<LoginWithOAuthApp />
150150
</MemoryRouter>
151151
</AppContext.Provider>,
152152
);
@@ -172,7 +172,7 @@ describe('routes/LoginEnterprise.tsx', () => {
172172
render(
173173
<AppContext.Provider value={{ accounts: mockAccounts }}>
174174
<MemoryRouter>
175-
<LoginEnterpriseRoute />
175+
<LoginWithOAuthApp />
176176
</MemoryRouter>
177177
</AppContext.Provider>,
178178
);

0 commit comments

Comments
 (0)