Skip to content

Commit 25f5478

Browse files
committed
Fix review comments
1 parent cf8d55e commit 25f5478

7 files changed

Lines changed: 139 additions & 12 deletions

File tree

openam-ui/openam-ui-js-sdk/src/lib/Home.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,17 @@ import type { UserService } from "./userService";
2121
export default function Home({userService}:{userService: UserService}) {
2222

2323
const navigate = useNavigate();
24-
const init = async () => {
24+
useEffect(() => {
25+
const init = async () => {
2526
const userData = await userService.getUserIdFromSession()
2627
if (!userData || !userData.id) {
2728
navigate('/login')
2829
} else {
2930
navigate('/user')
3031
}
3132
}
32-
useEffect(() => {
3333
init();
34-
}, []);
34+
}, [userService, navigate]);
3535

3636
return <h2>Loading</h2>;
3737
}

openam-ui/openam-ui-js-sdk/src/lib/Login.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ const Login: React.FC<LoginProps> = ({ loginService }) => {
8989
}
9090
initAuth();
9191

92-
}, [])
92+
}, [loginService, realm, service, navigate])
9393

9494
const setCallbackValue = (i: number, val: string | number) => {
9595
if (!authData) {

openam-ui/openam-ui-js-sdk/src/lib/components/DefaultCallbackElement.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { useEffect } from "react";
1818
import type { Callback } from "../types";
1919
import type { CallbackElement } from "./types";
2020

21-
const ScriptElement = (scriptText: string) => {
21+
const ScriptElement: React.FC<{ scriptText: string }> = ({ scriptText }) => {
2222
useEffect(() => {
2323
const script = document.createElement('script');
2424
script.innerHTML = scriptText;
@@ -28,7 +28,7 @@ const ScriptElement = (scriptText: string) => {
2828
document.body.removeChild(script);
2929
}
3030
};
31-
}, []);
31+
}, [scriptText]);
3232

3333
return null; // This component renders nothing in the DOM
3434
}
@@ -50,7 +50,7 @@ const DefaultCallbackElement: CallbackElement = ({ callback, setCallbackValue })
5050
case "2":
5151
return <p>{message}</p>
5252
case "4":
53-
return ScriptElement(message);
53+
return <ScriptElement scriptText={message} />;
5454
default:
5555
console.log(`unknown message type: ${messageType}`)
5656
return <></>;

openam-ui/openam-ui-js-sdk/src/lib/components/DefaultUserForm.test.tsx

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
import { describe, it, expect, vi, beforeEach } from 'vitest';
18-
import { render, screen, fireEvent } from '@testing-library/react';
18+
import { render, screen, fireEvent, act } from '@testing-library/react';
1919
import DefaultUserForm from './DefaultUserForm';
2020
import { mockUserData } from '../__tests__/mocks';
2121
import type { UserData } from '../types';
@@ -149,6 +149,54 @@ describe('DefaultUserForm', () => {
149149
expect(defaultProps.savePasswordHandler).toHaveBeenCalledTimes(1);
150150
});
151151

152-
152+
describe('Change Password button accessibility', () => {
153+
it('has role="button" for assistive technology', () => {
154+
render(<DefaultUserForm {...defaultProps} />);
155+
156+
const changePasswordButton = screen.getByRole('button', { name: 'Change Password' });
157+
expect(changePasswordButton).toBeInTheDocument();
158+
});
159+
160+
it('is focusable via keyboard (tabIndex=0)', () => {
161+
render(<DefaultUserForm {...defaultProps} />);
162+
163+
const changePasswordButton = screen.getByRole('button', { name: 'Change Password' });
164+
expect(changePasswordButton).toHaveAttribute('tabindex', '0');
165+
});
166+
167+
it('opens password modal on Enter key press', () => {
168+
render(<DefaultUserForm {...defaultProps} />);
169+
170+
act(() => {
171+
fireEvent.keyDown(screen.getByRole('button', { name: 'Change Password' }), { key: 'Enter' });
172+
});
173+
174+
expect(screen.getByLabelText('New:')).toBeInTheDocument();
175+
expect(screen.getByLabelText('Confirm:')).toBeInTheDocument();
176+
expect(screen.getByText('Update Password')).toBeInTheDocument();
177+
});
178+
179+
it('opens password modal on Space key press', () => {
180+
render(<DefaultUserForm {...defaultProps} />);
181+
182+
act(() => {
183+
fireEvent.keyDown(screen.getByRole('button', { name: 'Change Password' }), { key: ' ' });
184+
});
185+
186+
expect(screen.getByLabelText('New:')).toBeInTheDocument();
187+
expect(screen.getByLabelText('Confirm:')).toBeInTheDocument();
188+
expect(screen.getByText('Update Password')).toBeInTheDocument();
189+
});
190+
191+
it('does not open password modal on other key presses', () => {
192+
render(<DefaultUserForm {...defaultProps} />);
193+
194+
act(() => {
195+
fireEvent.keyDown(screen.getByRole('button', { name: 'Change Password' }), { key: 'Tab' });
196+
});
197+
198+
expect(screen.queryByLabelText('New:')).not.toBeInTheDocument();
199+
});
200+
});
153201

154202
});

openam-ui/openam-ui-js-sdk/src/lib/components/DefaultUserForm.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ const DefaultUserForm: UserForm = ({ userData, setUserData, saveHandler, savePas
8282
</div>
8383
<div
8484
className="change-password-link"
85+
role="button"
86+
tabIndex={0}
8587
onClick={() => setIsPasswordModalOpen(true)}
88+
onKeyDown={(e) => (e.key === "Enter" || e.key === " ") && setIsPasswordModalOpen(true)}
8689
style={{ marginBottom: "1.5rem" }} // Inline style to ensure spacing from button
8790
>
8891
Change Password

openam-ui/openam-ui-js-sdk/src/lib/loginService.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class LoginService {
5959
} catch (e) {
6060
if(import.meta.env.MODE === 'development') {
6161
console.log("fallback to test data", e)
62-
return JSON.parse(authError) as AuthResponse;
62+
return JSON.parse(authDataJSON) as AuthResponse;
6363
} else {
6464
throw e
6565
}
@@ -205,5 +205,81 @@ const successfulAuth = `{
205205

206206
const authError = `{"code":401,"reason":"Unauthorized","message":"Authentication Failed"}`
207207

208+
const authDataJSON = `{
209+
"authId": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJvdGsiOiJsa21mODI5dHEzbmhraDNyNmVsbGZtYWpybCIsInJlYWxtIjoiZGM9b3BlbmFtLGRjPW9wZW5pZGVudGl0eXBsYXRmb3JtLGRjPW9yZyIsInNlc3Npb25JZCI6IkFRSUM1d00yTFk0U2ZjekloNTRQLTZ1czRod0tSa09ibWFKa251U0p3SUxNYi1VLipBQUpUU1FBQ01ERUFBbE5MQUJNMk56VTVOVEF5T1RrNU5UUXpOemM0T1RZNEFBSlRNUUFBKiJ9.0lYgF063co7bcg_-xbabvrZponm7NMq3s-IeYPaf9Js",
210+
"template": "",
211+
"stage": "DataStore1",
212+
"header": "Sign in to OpenAM",
213+
"infoText": [
214+
"",
215+
""
216+
],
217+
"callbacks": [
218+
{
219+
"type": "NameCallback",
220+
"output": [
221+
{
222+
"name": "prompt",
223+
"value": "User Name:"
224+
}
225+
],
226+
"input": [
227+
{
228+
"name": "IDToken1",
229+
"value": "demo"
230+
}
231+
]
232+
},
233+
{
234+
"type": "PasswordCallback",
235+
"output": [
236+
{
237+
"name": "prompt",
238+
"value": "Password:"
239+
}
240+
],
241+
"input": [
242+
{
243+
"name": "IDToken2",
244+
"value": "changeit"
245+
}
246+
]
247+
},
248+
{
249+
"type": "ConfirmationCallback",
250+
"output": [
251+
{
252+
"name": "prompt",
253+
"value": ""
254+
},
255+
{
256+
"name": "messageType",
257+
"value": 0
258+
},
259+
{
260+
"name": "options",
261+
"value": [
262+
"Register device",
263+
"Skip this step"
264+
]
265+
},
266+
{
267+
"name": "optionType",
268+
"value": -1
269+
},
270+
{
271+
"name": "defaultOption",
272+
"value": 0
273+
}
274+
],
275+
"input": [
276+
{
277+
"name": "IDToken3",
278+
"value": "1"
279+
}
280+
]
281+
}
282+
]
283+
}`
208284

209285
export { LoginService }

openam-ui/openam-ui-js-sdk/src/lib/userService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class UserService {
5050
if (import.meta.env.MODE === 'development') {
5151
console.log("error getting user id from session", e)
5252
console.log("fallback to demo user")
53-
return JSON.parse(usersSuccessfulResponse)
53+
return JSON.parse(userUnauthorizedResponse)
5454
} else {
5555
console.log("request error occurred:", e)
5656
}
@@ -178,7 +178,7 @@ const usersSuccessfulResponse = `{
178178
"fullLoginURL": "/openam/UI/Login?realm=%2F"
179179
}`
180180

181-
// const userUnauthorizedResponse = `{"code":401,"reason":"Unauthorized","message":"Access Denied"}`
181+
const userUnauthorizedResponse = `{"code":401,"reason":"Unauthorized","message":"Access Denied"}`
182182

183183
const testUserData = `{
184184
"username": "demo",

0 commit comments

Comments
 (0)