Improve UX and functionality of the secret share page #3327
Conversation
WalkthroughThe pull request introduces enhancements to the secret-sharing functionality in two components. The Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (2)
85-94: Consider modifying the secretModified logicThe current implementation sets
secretModifiedto true when the secret field has a value, but doesn't account for when a user intentionally clears the field after initial input.const handleSecretChange = (e: ChangeEvent<HTMLTextAreaElement>) => { - if (e.target.value) { - setSecretModified(true); - } + setSecretModified(true); };
327-328: Consider consolidating default valuesThe default values for
expiresInandviewLimitare set via thedefaultValueprop, but it would be more consistent to set them in thedefaultValuesobject in theuseFormhook.const { control, reset, handleSubmit, watch, formState: { isSubmitting } } = useForm<FormData>({ resolver: zodResolver(schema), defaultValues: { secret: value || "", + expiresIn: DEFAULT_EXPIRES_IN, + viewLimit: DEFAULT_VIEW_LIMIT, + ...(isPublic ? {} : { accessType: SecretSharingAccessType.Organization }) }, mode: "onChange" });Then update the Controller components:
<Controller control={control} name="expiresIn" - defaultValue={DEFAULT_EXPIRES_IN} render={...} /> <Controller control={control} name="viewLimit" - defaultValue={DEFAULT_VIEW_LIMIT} render={...} />Also applies to: 343-344
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx(11 hunks)
🔇 Additional comments (11)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (11)
30-31: Good use of constants instead of magic numbersReplacing hardcoded values with named constants improves code readability and maintainability.
56-58: Well-structured state management for new featuresThe new state variables clearly define their purpose:
isSecretVisiblefor controlling secret visibilitypasswordConfirmationfor tracking the confirmation fieldsecretModifiedfor validating only after user interactionThis approach creates a better user experience by providing appropriate validation timing.
82-84: Good use of form watchingUsing
watchto monitor form fields enables reactive UI updates based on field values, which is necessary for the password confirmation and secret visibility features.
95-98: Good implementation of secret maskingThe computed value properly handles both visible and hidden states, including edge cases where the secret might be empty.
99-104: Good form validation logicThe validation variables clearly define various states and edge cases for password validation, which improves user experience by providing immediate feedback.
129-136: Improved form reset implementationThe form reset logic properly clears all relevant fields and resets to default values after submission, enhancing the user experience when creating multiple secrets.
191-234: Great implementation of secret visibility toggle and copy protectionThis implementation addresses several important UX concerns:
- Allows users to hide sensitive data to prevent shoulder surfing
- Prevents copying hidden text via keyboard shortcuts and context menu
- Maintains a clear visual indication of the current visibility state
This aligns perfectly with the PR objectives.
257-262: Good UX consideration for password fieldsAutomatically clearing the confirmation field when the password is emptied prevents confusion and maintains consistency between the fields.
268-322: Excellent password confirmation implementationThe password confirmation field implementation:
- Dynamically appears only when needed
- Provides excellent visual feedback with color-coded borders and icons
- Shows helpful error messages for mismatched passwords
- Uses smooth animations for better user experience
This is a significant UX improvement.
380-381: Nice dynamic button stylingChanging the button variant based on its disabled state provides clear visual feedback to users about form validity.
407-419: Comprehensive state reset when sharing another secretThe button handler properly resets all state variables and form fields, ensuring a clean slate for creating another secret.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (1)
115-115: Bug: Potential type inconsistency.Converting
expiresInfrom string to number should be done consistently. Consider using Number() at the point where the value is stored rather than at the point of use.- const expiresAt = new Date(Date.now() + Number(expiresIn)); + const numericExpiresIn = Number(expiresIn); + const expiresAt = new Date(Date.now() + numericExpiresIn);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx(11 hunks)
🔇 Additional comments (13)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (13)
30-31: Good addition of constants to replace magic numbers!Using constants for default values (DEFAULT_EXPIRES_IN and DEFAULT_VIEW_LIMIT) improves code maintainability and readability by eliminating magic numbers.
56-58: Well-structured state variables for the new features.These new state variables effectively support the UX improvements mentioned in the PR objectives:
isSecretVisiblecontrols the secret visibility togglepasswordConfirmationenables password validationsecretModifiedhelps with proper form validation feedback
85-87: Good use of form watching.Using
watchis the correct approach to respond to form field changes without requiring additional state variables for these values.
88-95: Clean implementation of event handlers.The handlers effectively track whether the secret has been modified, which is important for proper validation feedback.
96-104: Well-structured computed values for validation.The computed values provide clear conditions for various UI states:
- Secret visibility display
- Password validation logic
- Submit button state
This approach keeps the template code clean and easy to understand.
122-122: Good use of undefined for default value.Using
undefinedwhen the view limit is set to default is a clean approach that avoids unnecessary processing.
129-136: Proper form reset implementation.The form reset logic now properly resets all fields including the new password confirmation field. Using the constants for default values ensures consistency.
187-188: Improved error handling for secret field.Using
secretModifiedto conditionally show errors prevents premature error messages before the user has interacted with the field, improving the user experience.
192-238: Great implementation of the secret visibility toggle!This implementation successfully addresses the PR objective to allow hiding secrets. Key features:
- Toggle button with appropriate icons
- Asterisk masking for hidden secrets
- Prevention of copy/paste when hidden (security feature)
- Context menu prevention when hidden (security feature)
These security measures help prevent shoulder surfing as mentioned in the PR objectives.
262-267: Good UX improvement in password field.Automatically clearing the password confirmation when the password field is emptied ensures the fields stay in sync and prevents confusion.
273-326: Excellent password confirmation implementation!This implementation provides:
- Smooth animation for showing/hiding the field
- Clear visual feedback with color-coded borders
- Helpful validation icons
- Informative error messages for mismatched passwords
The conditional rendering based on password presence is a good UX pattern.
404-405: Good visual feedback for button state.Changing both the disabled state and variant of the button based on validation provides clear visual feedback to users about when they can submit the form.
431-443: Complete reset functionality for sharing another secret.The reset logic properly handles all state variables and form fields, ensuring a clean slate for the next secret.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (1)
296-304: Consider extracting border color logic to a separate function.The conditional border color logic is complex and nested within the JSX. It would be clearer and more maintainable if extracted to a separate function.
-containerClassName={ - password && passwordConfirmation - ? (password === passwordConfirmation ? "border-green-500" : "border-red-500") - : (password && !passwordConfirmation) - ? "border-amber-500 animate-pulse" - : "border-mineshaft-600" -} +containerClassName={getPasswordConfirmationBorderClass(password, passwordConfirmation)}Add this function before the return statement:
const getPasswordConfirmationBorderClass = (pwd: string, pwdConfirm: string) => { if (pwd && pwdConfirm) { return pwd === pwdConfirm ? "border-green-500" : "border-red-500"; } if (pwd && !pwdConfirm) { return "border-amber-500 animate-pulse"; } return "border-mineshaft-600"; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx(11 hunks)
🔇 Additional comments (16)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (16)
3-3: Added FontAwesome icons for enhanced UI functionality.The addition of
faEye,faEyeSlash, andfaExclamationTriangleicons enhances the user interface by providing visual indicators for secret visibility and validation states.
30-31: Good practice: Magic numbers replaced with named constants.Replacing magic numbers with named constants improves code readability and maintainability. The default values for expire time and view limit are now clearly defined.
56-58: New state variables enhance user experience.The addition of these state variables improves functionality:
isSecretVisiblecontrols whether the secret is displayed or hiddenpasswordConfirmationprovides validation for passwordssecretModifiedtracks user interaction for appropriate error messagingThese changes directly support the PR objectives of improving UX and enhancing security.
72-72: Form field monitoring improves interactive validation.Using the
watchfunction to monitor password and secret fields enables real-time validation and UI updates, creating a more responsive user experience.Also applies to: 85-86
88-95: Handlers tracking user interaction improve form feedback.These handlers ensure the application knows when a user has interacted with the secret field, which allows for better timing of validation messages and improves the overall form experience.
96-99: Secret visibility toggle implemented correctly.The computed displayed secret logic correctly implements the "hide secret" functionality mentioned in the PR objectives, preventing shoulder surfing by masking the secret content when toggled.
100-103: Form validation logic ensures data integrity.The password validation logic is comprehensive, checking for:
- Password matching
- Incomplete password fields
- Presence of secret content
This improves the quality of submitted data and prevents potential user errors.
114-116: Improved type conversion for form values.Explicit conversion of string values to numbers ensures correct data types are used when creating the shared secret, preventing potential type-related bugs.
Also applies to: 122-122
129-136: Complete form reset after submission.The form reset now properly handles all fields, including the new password confirmation field, ensuring a clean slate for creating a new secret.
192-238: Enhanced secret input with visibility toggle and security features.The secret textarea now includes:
- A visibility toggle button
- Prevention of copy/paste when the secret is hidden
- Disabled context menu when the secret is hidden
- Visual feedback through border highlighting
These changes directly fulfill the PR objectives of enhancing security and improving UX.
213-226: Security enhancement: Prevented copy/paste of hidden secrets.The implementation correctly prevents both keyboard shortcuts and context menu actions for copying and pasting when the secret is hidden, effectively addressing the security improvement mentioned in the PR objectives.
262-267: Password field clears confirmation when emptied.When the password field is emptied, the confirmation field is also cleared, maintaining consistency in the form state and preventing validation confusion.
273-326: Password confirmation provides excellent visual feedback.The implemented password confirmation field:
- Appears/disappears dynamically based on password field content
- Uses color-coded borders to indicate match status
- Shows appropriate icons for different states
- Provides clear error messages
This implementation significantly improves form validation UX.
404-405: Dynamic submit button state improves user feedback.The submit button now dynamically changes its appearance and disabled state based on form validation, providing clear feedback about when the form can be submitted.
431-443: Complete state reset when sharing another secret.The "Share Another Secret" button now properly resets all form fields and state variables, ensuring a clean experience when users want to share multiple secrets.
204-208:❓ Verification inconclusive
Verify field update logic when secret is not visible.
When
isSecretVisibleis false, the field's value change handler doesn't execute, which is intentional for preventing editing while hidden. However, you might want to ensure users understand they need to make the secret visible before editing.
🏁 Script executed:
#!/bin/bash # Check if there are any UI messages or tooltips explaining how to edit when secret is hidden rg -A 3 -B 3 "isSecretVisible.*false" --glob "*.tsx" --glob "*.ts"Length of output: 68
Clarify User Guidance for Editing Hidden Secret
The code intentionally skips updating the field when
isSecretVisibleis false, preventing users from editing while the secret is hidden. However, our automated search did not reveal any UI messages or tooltips informing users that they must unhide the secret to enable editing. Please manually verify that the UI provides clear guidance to users under this condition. If not, consider adding an explicit message or tooltip to improve the UX.
- Location:
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx(lines 204–208)- Key Concern: Ensure that when the secret is hidden, users are informed they need to make it visible before editing.
beaukuhn
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (1)
99-107: Well-structured validation logic and computed properties.Breaking down complex conditionals into named boolean variables significantly improves code readability. This approach makes the validation logic easier to understand and maintain.
However, there's a redundant Boolean cast that can be simplified.
- const isPasswordConfirmationMissing = Boolean(password) && !Boolean(passwordConfirmation); + const isPasswordConfirmationMissing = Boolean(password) && !passwordConfirmation;🧰 Tools
🪛 Biome (1.9.4)
[error] 103-103: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx(11 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx
[error] 103-103: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (10)
frontend/src/pages/public/ShareSecretPage/components/ShareSecretForm.tsx (10)
30-31: Good use of constants instead of magic numbers.Using constants to replace magic numbers improves code readability and maintainability. Creating
DEFAULT_EXPIRES_INandDEFAULT_VIEW_LIMITmakes the code easier to understand and modify in the future.
56-58: Well-structured state variables for enhanced functionality.The addition of these state variables reflects good React patterns:
isSecretVisibleprovides toggle functionality for the secret's visibilitypasswordConfirmationenables password matching validationsecretModifiedadds better UX by waiting until user interaction before showing validation errorsThese align perfectly with the PR's objective to improve UX and security.
85-85: Effective use of React Hook Form's 'watch' functionality.Using
watchto track multiple form fields is more efficient than implementing multiple event handlers. This approach reduces boilerplate and keeps the component's state in sync with the form.
87-94: Good event handling design.Separating the event handlers for blur and change provides clear separation of concerns. Setting
secretModifiedonly after user interaction prevents premature validation errors, improving user experience.
95-97: Clever implementation of secret masking.The computed value approach for displaying either the actual secret or masked characters is elegant and effective. This implementation successfully addresses the PR objective of hiding sensitive information to prevent shoulder surfing.
196-242: Excellent implementation of the secret visibility toggle and copy protection.This implementation successfully addresses multiple PR objectives:
- The eye icon toggle for showing/hiding the secret
- Prevention of copying hidden text via keyboard shortcuts
- Prevention of copying hidden text via right-click context menu
The relative positioning of the toggle button is also well-implemented for a clean UI.
266-271: Smart UX improvement for password field.Clearing the password confirmation when the password field is emptied prevents confusion and ensures the fields stay in sync. This is a thoughtful enhancement to the form's usability.
277-331: Excellent implementation of the confirm password field.This implementation addresses a key PR objective with several nice features:
- Smooth animation when showing/hiding the confirmation field
- Clear visual indicators for field state (amber for missing, red for mismatch, green for match)
- Helpful error messages and icons that guide the user
The conditional styling provides excellent visual feedback to users, making the form more intuitive.
409-410: Enhanced submit button feedback.Disabling the submit button when validation fails provides immediate feedback to users. The variant change also provides visual cues about form validity, which improves overall UX.
436-448: Comprehensive reset functionality.The enhanced reset logic ensures all state variables are properly reset when sharing another secret. This prevents state leakage between form submissions and maintains a consistent user experience.
Description 📣
Improves the UX and functionality of the secret share page:
Google.Chrome.-.28.March.2025.mp4
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippetsSummary by CodeRabbit