Skip to content

Fixed issue #342 -- Add hide/unhide button for passwords in change password #352

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.

Sign up for GitHub

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

Open
Alien501 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fixed issue #342 -- Add hide/unhide button for passwords in change password #352

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e459724
Fixed issue #342 -- Add hide/unhide button for passwords in change pa…
Alien501 Apr 3, 2025
544df63
Replaced icons with the one used in signup page
Alien501 Apr 5, 2025
f628812
Removed space
Alien501 Apr 5, 2025
7ec27ac
Space should be removed now
Alien501 Apr 5, 2025
98f8864
Changed format
Alien501 Apr 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions frontend/src/containers/Users/ChangePasswordForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@ import { CHANGE_PASSWORD_FORM_SCHEMA } from "@/constants/schema";
import { getAccessToken } from '@/utils/getAccesstoken';
import { enqueueSnackbar } from 'notistack';
import { useState, useEffect } from 'react';
import { ChangePasswordFormType } from './ChangePasswordForm.types';
import type { ChangePasswordFormType } from './ChangePasswordForm.types';
import { CHANGE_PASSWORD_FORM_FIELDS } from './ChangePasswordForm.config';
import { FaEye, FaEyeSlash } from 'react-icons/fa';

const ChangePasswordForm = () => {
const [accessToken, setAccessToken] = useState<string | null>(null);
const [showPassword, setShowPassword] = useState({
newPassword: false,
confirmPassword: false
});

useEffect(() => {
const fetchAccessToken = async () => {
Expand Down Expand Up @@ -74,6 +79,10 @@ const ChangePasswordForm = () => {
}
};

const togglePasswordVisibility = (field: 'newPassword' | 'confirmPassword') => {
setShowPassword((prev) => ({ ...prev, [field]: !prev[field] }));
}

return (
<Card className="mx-auto space-y-6 bg-transparent border-none shadow-none w-full">
<CardHeader className="px-0">
Expand All @@ -90,7 +99,7 @@ const ChangePasswordForm = () => {
>
<form onSubmit={handleSubmit(onSubmit)} className="space-y-4">
{CHANGE_PASSWORD_FORM_FIELDS.map((field) => (
<div key={field.name} className="grid w-1/2 items-center gap-1.5">
<div key={field.name} className="grid w-1/2 items-center gap-1.5 relative">
<FormField
control={control}
name={field.name}
Expand All @@ -101,11 +110,30 @@ const ChangePasswordForm = () => {
</FormLabel>
<FormControl>
<Input
type={field.type}
placeholder={field.placeholder}
{...formField}
type={
field.name === 'newPassword' && showPassword.newPassword
?
'text' : field.name === 'confirmPassword' && showPassword.confirmPassword
?
'text' :
field.type
}
placeholder={field.placeholder}
{...formField}
/>
</FormControl>
<Button
className='absolute right-1 top-1/2 -translate-y-1/4 bg-transparent text-foreground hover:bg-transparent mt-2 w-max'
onClick={(e) => {e.preventDefault(); togglePasswordVisibility(field.name)}}>
Copy link
Copy Markdown
Collaborator

@Saurabhsharma884 Saurabhsharma884 May 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to preventDefault here ?

Copy link
Copy Markdown
Author

@Alien501 Alien501 May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah ig, since it is nested inside a form there is chance of it to be considered to be a submit action when clicked on it. Can be replaced with type="button"

{field.name === 'newPassword'
? showPassword.newPassword
? <FaEyeSlash size={18} />
: <FaEye size={18} />
: showPassword.confirmPassword
? <FaEyeSlash size={18} />
: <FaEye size={18} />
}
</Button>
Comment on lines +125 to +136
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Sep 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add accessible name/state to the visibility toggle button.

The icon-only button currently has no accessible name or pressed state, so screen-reader users will encounter an unlabeled control and cannot tell whether the password is visible. Please surface an accessible label that reflects the action (show/hide) and expose the toggle state via aria-pressed. Once you make it a plain type="button", the extra preventDefault handler is no longer necessary.

Apply this diff to address the issue:

-                                            <Button
-                                                className='absolute right-1 top-1/2 -translate-y-1/4 bg-transparent text-foreground hover:bg-transparent mt-2 w-max'
-                                                onClick={(e) => {e.preventDefault(); togglePasswordVisibility(field.name)}}>
+                                            <Button
+                                                type="button"
+                                                aria-label={
+                                                    field.name === 'newPassword'
+                                                        ? (showPassword.newPassword ? 'Hide new password' : 'Show new password')
+                                                        : (showPassword.confirmPassword ? 'Hide confirm password' : 'Show confirm password')
+                                                }
+                                                aria-pressed={
+                                                    field.name === 'newPassword'
+                                                        ? showPassword.newPassword
+                                                        : showPassword.confirmPassword
+                                                }
+                                                className='absolute right-1 top-1/2 -translate-y-1/4 bg-transparent text-foreground hover:bg-transparent mt-2 w-max'
+                                                onClick={() => togglePasswordVisibility(field.name)}>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
className='absolute right-1 top-1/2 -translate-y-1/4 bg-transparent text-foreground hover:bg-transparent mt-2 w-max'
onClick={(e) => {e.preventDefault(); togglePasswordVisibility(field.name)}}>
{field.name === 'newPassword'
? showPassword.newPassword
? <FaEyeSlash size={18} />
: <FaEye size={18} />
: showPassword.confirmPassword
? <FaEyeSlash size={18} />
: <FaEye size={18} />
}
</Button>
<Button
type="button"
aria-label={
field.name === 'newPassword'
? (showPassword.newPassword ? 'Hide new password' : 'Show new password')
: (showPassword.confirmPassword ? 'Hide confirm password' : 'Show confirm password')
}
aria-pressed={
field.name === 'newPassword'
? showPassword.newPassword
: showPassword.confirmPassword
}
className='absolute right-1 top-1/2 -translate-y-1/4 bg-transparent text-foreground hover:bg-transparent mt-2 w-max'
onClick={() => togglePasswordVisibility(field.name)}>
{field.name === 'newPassword'
? showPassword.newPassword
? <FaEyeSlash size={18} />
: <FaEye size={18} />
: showPassword.confirmPassword
? <FaEyeSlash size={18} />
: <FaEye size={18} />
}
</Button>
🤖 Prompt for AI Agents 
In frontend/src/containers/Users/ChangePasswordForm.tsx around lines 125 to 136,
the icon-only visibility toggle button lacks an accessible name and state;
change the Button to type="button", remove the unnecessary e.preventDefault()
call, add a dynamic aria-label that reflects the action ("Show password" or
"Hide password") based on the current visibility for the given field, and set
aria-pressed to a boolean representing whether the password is currently visible
(true when visible, false when hidden) so screen readers receive both label and
toggle state.

{errors[field.name] && (
<FormMessage>{errors[field.name]?.message}</FormMessage>
)}
Expand Down