Skip to content

feat: 2fa backup codes#2

Draft
ShashankFC wants to merge 1 commit into
enhance-two-factor-security-foundationfrom
improve-two-factor-authentication-features
Draft

feat: 2fa backup codes#2
ShashankFC wants to merge 1 commit into
enhance-two-factor-security-foundationfrom
improve-two-factor-authentication-features

Conversation

@ShashankFC

Copy link
Copy Markdown
Collaborator

Test 3nn---nReplicated from ai-code-review-evaluation/cal.com-cursor#3

Co-authored-by: Peer Richelsen <peeroke@gmail.com>
@GitHoobar

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a308075bc3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +266 to +269
onClick={(e) => {
e.preventDefault();
resetState();
onEnable();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Call onEnable when backup-code dialog is dismissed

In the backup-code step, onEnable() is only invoked from the “Close” button handler. If the user dismisses the dialog via the overlay or Esc (Radix Dialog defaults), this handler never runs, so the parent does not invalidate viewer.me and the settings UI can stay in the “2FA disabled” state even though the enable endpoint already returned 200 and advanced to DisplayBackupCodes. That also lets users accidentally close the modal without saving the codes. Consider triggering onEnable/resetState from onOpenChange when step === DisplayBackupCodes or disabling outside-close for that step.

Useful? React with 👍 / 👎.

Comment on lines +47 to +51
<button
className="text-emphasis h-9"
tabIndex={-1}
type="button"
onClick={() => toggleIsPasswordVisible()}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 Badge Keep password visibility toggle keyboard-accessible

Setting tabIndex={-1} on the password visibility toggle removes it from the tab order, which means keyboard-only users can no longer reach the control to show/hide the password. This is an accessibility regression from the previous behavior where the button was focusable. If there was a focus/UX issue you were addressing, consider alternative fixes (e.g., styling or focus management) that keep the button keyboard-accessible.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants