Skip to content

refactor: use solid simple modals (@fehmer)#8044

Open
fehmer wants to merge 18 commits into
masterfrom
feature/migrate-simple-modals
Open

refactor: use solid simple modals (@fehmer)#8044
fehmer wants to merge 18 commits into
masterfrom
feature/migrate-simple-modals

Conversation

@fehmer
Copy link
Copy Markdown
Member

@fehmer fehmer commented Jun 2, 2026

No description provided.

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Jun 2, 2026
Comment thread frontend/src/ts/components/modals/account-settings/RemoveAuthMethodModal.tsx Outdated
Comment thread frontend/src/ts/components/modals/account-settings/RemoveAuthMethodModal.tsx Outdated
Copy link
Copy Markdown
Contributor

@Leonabcd123 Leonabcd123 left a comment

Choose a reason for hiding this comment

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

For your consideration, more of a style comment.

Comment thread frontend/src/ts/components/modals/account-settings/RemoveAuthMethodModal.tsx Outdated
@fehmer fehmer marked this pull request as ready for review June 2, 2026 21:35
@github-actions github-actions Bot added the waiting for review Pull requests that require a review before continuing label Jun 2, 2026
import { PasswordSchema } from "@monkeytype/schemas/users";
import { z, ZodString } from "zod";

export type AuthMethod = "password" | "github.com" | "google.com";
Copy link
Copy Markdown
Contributor

@Leonabcd123 Leonabcd123 Jun 4, 2026

Choose a reason for hiding this comment

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

I think we should first define an array with all of these values, then infer a union type out of it. This will remove

const authMethods = ["password", "github.com", "google.com"] as AuthMethod[];

in getPreferredAuthenticationMethod, which would make it easier to expand authMethods (only need to change the array instead of both the array and the type).

Suggested change
export type AuthMethod = "password" | "github.com" | "google.com";
const authMethods = ["password", "github.com", "google.com"] as const;
export type AuthMethod = typeof authMethods[number];

And remove authMethods definition in getPreferredAuthenticationMethod.

Comment on lines +30 to +34
const hasRemainingAuth = [
isUsingPasswordAuthentication() && options.authMethod !== "password",
isUsingGithubAuthentication() && options.authMethod !== "github.com",
isUsingGoogleAuthentication() && options.authMethod !== "google.com",
].find((it) => it);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Depends on previous review comment.

Suggested change
const hasRemainingAuth = [
isUsingPasswordAuthentication() && options.authMethod !== "password",
isUsingGithubAuthentication() && options.authMethod !== "github.com",
isUsingGoogleAuthentication() && options.authMethod !== "google.com",
].find((it) => it);
const hasRemainingAuth = authMethods.some((authMethod) => isUsingAuthentication(authMethod) && options.authMethod !== authMethod);

Need to export isUsingAuthentication and authMethods, also remove isUsingGithubAuthentication and isUsingGoogleAuthentication.

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

Labels

frontend User interface or web stuff waiting for review Pull requests that require a review before continuing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants