Skip to content

refactor: improve simple modals (@fehmer)#8000

Open
fehmer wants to merge 10 commits into
masterfrom
feature/simpler-modal
Open

refactor: improve simple modals (@fehmer)#8000
fehmer wants to merge 10 commits into
masterfrom
feature/simpler-modal

Conversation

@fehmer
Copy link
Copy Markdown
Member

@fehmer fehmer commented May 22, 2026

No description provided.

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label May 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes.

@github-actions github-actions Bot added the waiting for update Pull requests or issues that require changes/comments before continuing label May 22, 2026
@fehmer fehmer changed the title Feature/simpler modal refactor: improve simple modals (@fehmer) May 22, 2026
@fehmer fehmer marked this pull request as draft May 22, 2026 19:04
@github-actions github-actions Bot removed the waiting for update Pull requests or issues that require changes/comments before continuing label May 22, 2026
@fehmer fehmer marked this pull request as ready for review May 23, 2026 09:51
@github-actions github-actions Bot added the waiting for review Pull requests that require a review before continuing label May 23, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes.

@github-actions github-actions Bot added waiting for update Pull requests or issues that require changes/comments before continuing and removed waiting for review Pull requests that require a review before continuing labels May 23, 2026
Comment thread frontend/src/ts/components/ui/form/InputField.tsx Outdated
@github-actions github-actions Bot removed the waiting for update Pull requests or issues that require changes/comments before continuing label May 23, 2026
@Miodec Miodec requested a review from Copilot May 23, 2026 12:32
@github-actions github-actions Bot added the waiting for review Pull requests that require a review before continuing label May 23, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the Solid showSimpleModal API to be schema-driven: callers pass a Zod object schema plus a matching inputs map, and execFn receives the parsed object. Loader/notification handling moves from states/simple-modal.ts into the SimpleModal component, and the legacy class-based modal is decoupled by copying its input types into elements/simple-modal.ts. Several call sites migrate from array-based inputs with manual validation to schema/preprocess.

Changes:

  • New typed SimpleModalConfig<S> keyed by Zod schema with InputsFromSchema<S>; execution/loader/notification moved into SimpleModal.tsx; min/max derived from schemas via getMinAndMax/getDateMinAndMax and a new convertFn.
  • Migrated AddTagModal, QuoteSearchModal, leaderboard Navigation, FontFamily, ImportExport, Tags, Theme to schema+preprocess form, replacing manual parseInt/normalizeName and validation.isValid blocks.
  • Legacy elements/simple-modal.ts now owns its own input/config types; consumers (simple-modals.ts, ape-key-table.ts) re-imported TextInput/TextArea/PasswordInput from there. Added typedEntries util.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/src/ts/utils/misc.ts Adds typed Object.entries helper used by the new modal.
frontend/src/ts/states/simple-modal.ts Rewrites types to be schema-generic; removes loader/notification execution.
frontend/src/ts/elements/simple-modal.ts Duplicates legacy input/config types locally to decouple from Solid path.
frontend/src/ts/modals/simple-modals.ts Updates legacy imports to new location.
frontend/src/ts/elements/account-settings/ape-key-table.ts Updates TextArea import path.
frontend/src/ts/components/modals/SimpleModal.tsx Reworks form to object-shaped inputs; adds convertFn, schema-based min/max, in-component execFn handling.
frontend/src/ts/components/modals/AddTagModal.tsx Migrates to schema + preprocess: normalizeName.
frontend/src/ts/components/modals/QuoteSearchModal.tsx Replaces parseInt/NaN checks with Zod number schema.
frontend/src/ts/components/pages/leaderboard/Navigation.tsx Uses local z.number().int().safe().min(1) instead of PageNumberSchema.
frontend/src/ts/components/pages/settings/custom-setting/FontFamily.tsx Uses schema + normalizeName preprocess.
frontend/src/ts/components/pages/settings/custom-setting/ImportExport.tsx Switches inputs to schema/object form.
frontend/src/ts/components/pages/settings/custom-setting/Tags.tsx Schema-driven rename; drops manual loader-bar calls.
frontend/src/ts/components/pages/settings/custom-setting/Presets.tsx Drops manual loader-bar calls.
frontend/src/ts/components/pages/settings/custom-setting/Theme.tsx Schema-driven share/update modals; uses replaceUnderscoresWithSpaces/normalizeName.
frontend/src/ts/components/ui/form/InputField.tsx Forwards min/max to underlying input.
Comments suppressed due to low confidence (1)

frontend/src/ts/components/modals/SimpleModal.tsx:210

  • After the user types into a date/datetime-local input, handleChange(e.currentTarget.value) writes a string into the form state. On the next render, formatDate(props.field().state.value as Date) will pass that string to date-fns/format, which expects a Date/number and will throw "Invalid time value" (or render NaN). Previously the state was stored as a pre-formatted string so this worked, but with the new initVal ?? null defaulting to a Date object the state alternates between Date and string. Consider either storing the formatted string in getDefaultValues for date inputs (as before) or detecting the value type inside formatDate and skipping formatting when it's already a string.
        <input
          type={props.input.type}
          class={cn("w-full", props.input.class)}
          value={formatDate(props.field().state.value as Date)}
          disabled={props.input.disabled}
          {...getDateMinAndMax(props.schema, formatDate)}
          onInput={(e) => {
            props.field().handleChange(e.currentTarget.value);
            props.input.oninput?.(e);
          }}
          onBlur={() => props.field().handleBlur()}
        />

Comment on lines +59 to +61
return Object.fromEntries(
Object.entries(inputs).map(([key, input]) => [key, input.initVal ?? null]),
);
Comment on lines 68 to +69
const required =
!input.hidden && !input.optional && input.type !== "checkbox";

const schema = input.validation?.schema;
!input.hidden && !schema.isOptional() && input.type !== "checkbox";
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