Skip to content

Commit c116ebc

Browse files
authored
feat: add auth fallback logic for when official Coder components are not mounted (#128)
* wip: commit progress on fallback UI * chore: move dep to peer dependencies * wip: commit more progress * wip: more progress * refactor: consolidate card logic * fix: update component tracking hooks * fix: add a11y landmark to auth fallback * wip: commit more style progress * wip: commit more progress * wip: more progress * wip: cleanup current approach * wip: commit progress on observer approach * wip: fix infinite loop for mutation logic * fix: prevent padding patches from firing too often * fix: improve scoping of style overrides * chore: finish intial version of fallback stylling * fix: tidy up types * wip: create initial version of dialog form * wip: commit progress on modal * chore: finish styling for modal wrapper * fix: update padding for FormDialog * wip: start extracting out auth form * fix: add missing barrel export file * fix: make sure that auth form isn't dismissed early * fix: update auth imports * fix: update spacing for auth modal * refactor: clean up auth provider for clarity * docs: rewrite comment for clarity * fix: improve granularity between official Coder components and user components * fix: update all internal consumers of useCoderAuth * wip: commit initial version of useCoderQuery helper hook * refactor: rename hooks to avoid confusion * fix: update exports for plugin * docs: fill in incomplete sentence * wip: commit initial version of useMutation wrapper * refactor: extract retry factor into global constant * fix: add explicit return type to useCoderMutation * wip: start extracting auth logic into better reusable components * fix: update card to have better styling for body * wip: commit progress on style refactoring * fix: update vertical padding for card wrapper * chore: delete CoderAuthWrapper component * fix: update styling for auth fallback * chore: shrink size of PR * fix: update imports * docs: add comment about description setup * fix: remove risk of runtime render errors in auth form * fix: update imports * fix: update font sizes to use relative units * fix: update peer dependencies for react-dom * refactor: clean up auth revalidation logic * wip: start updating tests for new code changes * fix: adding missing test case for auth card * wip: commit progress on auth form test updates * fix: removal vetigal properties * fix: get all CoderAuthForm tests passing * fix: update import for auth hook in test
1 parent 2fd0959 commit c116ebc

31 files changed

+1002
-383
lines changed

plugins/backstage-plugin-coder/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@
4646
"valibot": "^0.28.1"
4747
},
4848
"peerDependencies": {
49-
"react": "^16.13.1 || ^17.0.0 || ^18.0.0"
49+
"react": "^16.13.1 || ^17.0.0 || ^18.0.0",
50+
"react-dom": "^16.13.1 || ^17.0.0 || ^18.0.0"
5051
},
5152
"devDependencies": {
5253
"@backstage/cli": "^0.25.1",
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* @file A slightly different take on Backstage's official InfoCard component,
3+
* with better support for accessibility.
4+
*
5+
* Does not support all of InfoCard's properties just yet.
6+
*/
7+
import React, { type HTMLAttributes, type ReactNode, forwardRef } from 'react';
8+
import { makeStyles } from '@material-ui/core';
9+
10+
export type A11yInfoCardProps = Readonly<
11+
HTMLAttributes<HTMLDivElement> & {
12+
headerContent?: ReactNode;
13+
}
14+
>;
15+
16+
const useStyles = makeStyles(theme => ({
17+
root: {
18+
color: theme.palette.type,
19+
backgroundColor: theme.palette.background.paper,
20+
padding: theme.spacing(2),
21+
borderRadius: theme.shape.borderRadius,
22+
boxShadow: theme.shadows[1],
23+
},
24+
25+
headerContent: {
26+
// Ideally wouldn't be using hard-coded font sizes, but couldn't figure out
27+
// how to use the theme.typography property, especially since not all
28+
// sub-properties have font sizes defined
29+
fontSize: '1.5rem',
30+
color: theme.palette.text.primary,
31+
fontWeight: 700,
32+
borderBottom: `1px solid ${theme.palette.divider}`,
33+
34+
// Margins and padding are a bit wonky to support full-bleed layouts
35+
marginLeft: `-${theme.spacing(2)}px`,
36+
marginRight: `-${theme.spacing(2)}px`,
37+
padding: `0 ${theme.spacing(2)}px ${theme.spacing(2)}px`,
38+
},
39+
}));
40+
41+
// Card should be treated as equivalent to Backstage's official InfoCard
42+
// component; had to make custom version so that it could forward properties for
43+
// accessibility/screen reader support
44+
export const A11yInfoCard = forwardRef<HTMLDivElement, A11yInfoCardProps>(
45+
(props, ref) => {
46+
const { className, children, headerContent, ...delegatedProps } = props;
47+
const styles = useStyles();
48+
49+
return (
50+
<div
51+
ref={ref}
52+
className={`${styles.root} ${className ?? ''}`}
53+
{...delegatedProps}
54+
>
55+
{headerContent !== undefined && (
56+
<div className={styles.headerContent}>{headerContent}</div>
57+
)}
58+
59+
{children}
60+
</div>
61+
);
62+
},
63+
);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './A11yInfoCard';

plugins/backstage-plugin-coder/src/components/Card/Card.tsx

Lines changed: 0 additions & 27 deletions
This file was deleted.

plugins/backstage-plugin-coder/src/components/Card/index.ts

Lines changed: 0 additions & 1 deletion
This file was deleted.

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthDistrustedForm.tsx renamed to plugins/backstage-plugin-coder/src/components/CoderAuthForm/CoderAuthDistrustedForm.tsx

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import React from 'react';
22
import { CoderLogo } from '../CoderLogo';
3-
import { LinkButton } from '@backstage/core-components';
43
import { makeStyles } from '@material-ui/core';
5-
import { useCoderAuth } from '../CoderProvider';
4+
import { UnlinkAccountButton } from './UnlinkAccountButton';
65

76
const useStyles = makeStyles(theme => ({
87
root: {
@@ -31,8 +30,6 @@ const useStyles = makeStyles(theme => ({
3130

3231
export const CoderAuthDistrustedForm = () => {
3332
const styles = useStyles();
34-
const { ejectToken } = useCoderAuth();
35-
3633
return (
3734
<div className={styles.root}>
3835
<div>
@@ -43,18 +40,7 @@ export const CoderAuthDistrustedForm = () => {
4340
</p>
4441
</div>
4542

46-
<LinkButton
47-
disableRipple
48-
to=""
49-
component="button"
50-
type="submit"
51-
color="primary"
52-
variant="contained"
53-
className={styles.button}
54-
onClick={ejectToken}
55-
>
56-
Eject token
57-
</LinkButton>
43+
<UnlinkAccountButton className={styles.button} />
5844
</div>
5945
);
6046
};

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.test.tsx renamed to plugins/backstage-plugin-coder/src/components/CoderAuthForm/CoderAuthForm.test.tsx

Lines changed: 19 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,14 @@ import {
88
mockAuthStates,
99
mockCoderAuthToken,
1010
} from '../../testHelpers/mockBackstageData';
11-
import { CoderAuthWrapper } from './CoderAuthWrapper';
11+
import { CoderAuthForm } from './CoderAuthForm';
1212
import { renderInTestApp } from '@backstage/test-utils';
1313

1414
type RenderInputs = Readonly<{
1515
authStatus: CoderAuthStatus;
16-
childButtonText?: string;
1716
}>;
1817

19-
async function renderAuthWrapper({
20-
authStatus,
21-
childButtonText = 'Default button text',
22-
}: RenderInputs) {
18+
async function renderAuthWrapper({ authStatus }: RenderInputs) {
2319
const ejectToken = jest.fn();
2420
const registerNewToken = jest.fn();
2521

@@ -40,67 +36,36 @@ async function renderAuthWrapper({
4036
*/
4137
const renderOutput = await renderInTestApp(
4238
<CoderProviderWithMockAuth appConfig={mockAppConfig} auth={auth}>
43-
<CoderAuthWrapper type="card">
44-
<button>{childButtonText}</button>
45-
</CoderAuthWrapper>
39+
<CoderAuthForm />
4640
</CoderProviderWithMockAuth>,
4741
);
4842

4943
return { ...renderOutput, ejectToken, registerNewToken };
5044
}
5145

52-
describe(`${CoderAuthWrapper.name}`, () => {
53-
describe('Displaying main content', () => {
54-
it('Displays the main children when the user is authenticated', async () => {
55-
const buttonText = 'I have secret Coder content!';
56-
renderAuthWrapper({
57-
authStatus: 'authenticated',
58-
childButtonText: buttonText,
59-
});
60-
61-
const button = await screen.findByRole('button', { name: buttonText });
62-
63-
// This assertion isn't necessary because findByRole will throw an error
64-
// if the button can't be found within the expected period of time. Doing
65-
// this purely to make the Backstage linter happy
66-
expect(button).toBeInTheDocument();
67-
});
68-
});
69-
46+
describe(`${CoderAuthForm.name}`, () => {
7047
describe('Loading UI', () => {
7148
it('Is displayed while the auth is initializing', async () => {
72-
const buttonText = "You shouldn't be able to see me!";
73-
renderAuthWrapper({
74-
authStatus: 'initializing',
75-
childButtonText: buttonText,
76-
});
77-
78-
await screen.findByText(/Loading/);
79-
const button = screen.queryByRole('button', { name: buttonText });
80-
expect(button).not.toBeInTheDocument();
49+
renderAuthWrapper({ authStatus: 'initializing' });
50+
const loadingIndicator = await screen.findByText(/Loading/);
51+
expect(loadingIndicator).toBeInTheDocument();
8152
});
8253
});
8354

8455
describe('Token distrusted form', () => {
8556
it("Is displayed when the user's auth status cannot be verified", async () => {
86-
const buttonText = 'Not sure if you should be able to see me';
8757
const distrustedTextMatcher = /Unable to verify token authenticity/;
8858
const distrustedStatuses: readonly CoderAuthStatus[] = [
8959
'distrusted',
9060
'noInternetConnection',
9161
'deploymentUnavailable',
9262
];
9363

94-
for (const status of distrustedStatuses) {
95-
const { unmount } = await renderAuthWrapper({
96-
authStatus: status,
97-
childButtonText: buttonText,
98-
});
99-
100-
await screen.findByText(distrustedTextMatcher);
101-
const button = screen.queryByRole('button', { name: buttonText });
102-
expect(button).not.toBeInTheDocument();
64+
for (const authStatus of distrustedStatuses) {
65+
const { unmount } = await renderAuthWrapper({ authStatus });
66+
const message = await screen.findByText(distrustedTextMatcher);
10367

68+
expect(message).toBeInTheDocument();
10469
unmount();
10570
}
10671
});
@@ -112,58 +77,28 @@ describe(`${CoderAuthWrapper.name}`, () => {
11277

11378
const user = userEvent.setup();
11479
const ejectButton = await screen.findByRole('button', {
115-
name: 'Eject token',
80+
name: /Unlink Coder account/,
11681
});
11782

11883
await user.click(ejectButton);
11984
expect(ejectToken).toHaveBeenCalled();
12085
});
121-
122-
it('Will appear if auth status changes during re-renders', async () => {
123-
const buttonText = "Now you see me, now you don't";
124-
const { rerender } = await renderAuthWrapper({
125-
authStatus: 'authenticated',
126-
childButtonText: buttonText,
127-
});
128-
129-
// Capture button after it first appears on the screen
130-
const button = await screen.findByRole('button', { name: buttonText });
131-
132-
rerender(
133-
<CoderProviderWithMockAuth
134-
appConfig={mockAppConfig}
135-
authStatus="distrusted"
136-
>
137-
<CoderAuthWrapper type="card">
138-
<button>{buttonText}</button>
139-
</CoderAuthWrapper>
140-
</CoderProviderWithMockAuth>,
141-
);
142-
143-
// Assert that the button is now gone
144-
expect(button).not.toBeInTheDocument();
145-
});
14686
});
14787

14888
describe('Token submission form', () => {
14989
it("Is displayed when the token either doesn't exist or is definitely not valid", async () => {
150-
const buttonText = "You're not allowed to gaze upon my visage";
151-
const tokenFormMatcher = /Please enter a new token/;
15290
const statusesForInvalidUser: readonly CoderAuthStatus[] = [
15391
'invalid',
15492
'tokenMissing',
15593
];
15694

157-
for (const status of statusesForInvalidUser) {
158-
const { unmount } = await renderAuthWrapper({
159-
authStatus: status,
160-
childButtonText: buttonText,
95+
for (const authStatus of statusesForInvalidUser) {
96+
const { unmount } = await renderAuthWrapper({ authStatus });
97+
const form = screen.getByRole('form', {
98+
name: /Authenticate with Coder/,
16199
});
162100

163-
await screen.findByText(tokenFormMatcher);
164-
const button = screen.queryByRole('button', { name: buttonText });
165-
expect(button).not.toBeInTheDocument();
166-
101+
expect(form).toBeInTheDocument();
167102
unmount();
168103
}
169104
});
@@ -178,7 +113,8 @@ describe(`${CoderAuthWrapper.name}`, () => {
178113
* 1. The auth input is of type password, which does not have a role
179114
* compatible with Testing Library; can't use getByRole to select it
180115
* 2. MUI adds a star to its labels that are required, meaning that any
181-
* attempts at trying to match the string "Auth token" will fail
116+
* attempts at trying to match string literal "Auth token" will fail;
117+
* have to use a regex selector
182118
*/
183119
const inputField = screen.getByLabelText(/Auth token/);
184120
const submitButton = screen.getByRole('button', { name: 'Authenticate' });

0 commit comments

Comments
 (0)