Skip to content

refactor: solid test config (@miodec)#7701

Draft
Miodec wants to merge 40 commits intomasterfrom
solid-test-settings
Draft

refactor: solid test config (@miodec)#7701
Miodec wants to merge 40 commits intomasterfrom
solid-test-settings

Conversation

@Miodec
Copy link
Member

@Miodec Miodec commented Mar 22, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 22, 2026 13:35
@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Mar 22, 2026
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Mar 22, 2026
fixedWidth: true,
}}
onClick={() => {
showErrorNotification("//todo");
Copy link
Contributor

Choose a reason for hiding this comment

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

A new Todo was discovered. If it is not a priority right now,consider marking it for later attention.
todo");

Copy link
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 test configuration + custom text tooling toward SolidJS + the central modal store, replacing several legacy HTML/imperative modals and introducing new Solid-based modals/components.

Changes:

  • Add Solid TestConfig mount + new Solid modals for custom text workflow (save/load/filter/generate) and test settings sharing.
  • Move “loaded challenge” state into a Solid signal store and update challenge/modes-notice code to use it.
  • Remove legacy popup HTML/CSS and legacy modal implementations for the migrated modals.

Reviewed changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pnpm-lock.yaml Lockfile updates from dependency graph changes (vite/vitest peer resolution).
frontend/src/ts/test/test-state.ts Removes legacy activeChallenge state from test state module.
frontend/src/ts/states/test.ts Adds Solid signal store for loaded challenge.
frontend/src/ts/states/modals.ts Extends modal ID union with new Solid modal IDs.
frontend/src/ts/states/custom-text-modal.ts Adds a custom-text modal state module (currently appears unused).
frontend/src/ts/modals/word-filter.ts Deletes legacy word filter modal implementation.
frontend/src/ts/modals/simple-modals.ts Removes legacy simple-modals entries for custom text actions.
frontend/src/ts/modals/simple-modals-base.ts Removes popup keys for deleted custom text simple-modals.
frontend/src/ts/modals/saved-texts.ts Deletes legacy saved texts modal implementation.
frontend/src/ts/modals/save-custom-text.ts Deletes legacy save custom text modal implementation.
frontend/src/ts/modals/mobile-test-config.ts Switches “custom text” entry point to the modal store (showModal("CustomText")).
frontend/src/ts/modals/custom-text.ts Deletes legacy custom text modal implementation.
frontend/src/ts/modals/custom-generator.ts Deletes legacy custom generator modal implementation.
frontend/src/ts/event-handlers/test.ts Routes custom text button to showModal("CustomText").
frontend/src/ts/elements/modes-notice.ts Reads challenge display from new loaded-challenge signal store.
frontend/src/ts/controllers/challenge-controller.ts Uses new loaded-challenge signal store instead of legacy test-state field.
frontend/src/ts/components/ui/SlimSelect.tsx Adds disabled prop + forces SlimSelect dropdown content to render within a wrapper container.
frontend/src/ts/components/pages/test/TestConfig.tsx New Solid test config component + actions to open new modals.
frontend/src/ts/components/mount.tsx Registers new testconfig mount component.
frontend/src/ts/components/modals/WordFilterModal.tsx New Solid word-filter modal that chains data back into custom text.
frontend/src/ts/components/modals/TestDuration.tsx New Solid “custom test duration” modal.
frontend/src/ts/components/modals/ShareTestSettings.tsx New Solid “share test settings” modal (builds a compressed URL).
frontend/src/ts/components/modals/SavedTextsModal.tsx New Solid saved-texts modal with delete/reset-progress actions via simple modal.
frontend/src/ts/components/modals/SaveCustomTextModal.tsx New Solid save custom text modal with zod + duplicate-name validation.
frontend/src/ts/components/modals/Modals.tsx Renders newly added Solid modals in the global modals container.
frontend/src/ts/components/modals/CustomWordAmount.tsx New Solid “custom word amount” modal.
frontend/src/ts/components/modals/CustomTextModal.tsx New Solid custom text modal + nested Solid modals for related workflows.
frontend/src/ts/components/modals/CustomGeneratorModal.tsx New Solid custom generator modal that chains generated text back.
frontend/src/ts/components/common/anime/AnimeShow.tsx Adds animeProps passthrough to customize Anime wrapper props.
frontend/src/ts/components/common/AnimatedModal.tsx Passes isChained into beforeShow hook for modal chaining logic.
frontend/src/ts/commandline/lists.ts Routes “Change custom text” command to showModal("CustomText").
frontend/src/styles/popups.scss Removes legacy CSS for deleted custom-text-related popups.
frontend/src/styles/media-queries-yellow.scss Removes legacy custom text modal responsive CSS.
frontend/src/styles/media-queries-purple.scss Removes legacy custom text/word filter responsive CSS.
frontend/src/styles/media-queries-green.scss Removes legacy custom text modal responsive CSS.
frontend/src/styles/media-queries-blue.scss Removes legacy custom text modal responsive CSS.
frontend/src/styles/inputs.scss Removes legacy exact-match checkbox sizing rule tied to deleted HTML modal.
frontend/src/html/popups.html Removes legacy popup HTML for custom text/saved texts/save/filter/generator modals.
frontend/src/html/pages/test.html Adds Solid testconfig mount while keeping legacy #testConfig markup.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

</div>
</Show>
<textarea
ref={textareaRef}
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

ref={textareaRef} won't assign the element to the local textareaRef variable in Solid; ref needs a callback (or a useRef* helper). As written, textareaRef will stay undefined, so afterShow won't focus the textarea.

Suggested change
ref={textareaRef}
ref={(el) => (textareaRef = el)}

Copilot uses AI. Check for mistakes.
Comment on lines +532 to +544
<input
ref={fileInputRef}
type="file"
class="hidden"
accept=".txt"
onChange={handleFileOpen}
/>
<Button
variant="button"
fa={{ icon: "fa-file-import" }}
text="open file"
onClick={() => fileInputRef.click()}
/>
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

ref={fileInputRef} has the same issue as textareaRef: Solid won't assign the DOM node into this local variable. This will cause runtime errors when calling fileInputRef.click() / reading fileInputRef.files. Use a callback ref (or useRefWithUtils/useRef).

Copilot uses AI. Check for mistakes.
CustomTextPopup.show({
modalChain: modal,
});
showModal("CustomText");
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

mobileTestConfigModal is a legacy utils/animated-modal instance (not store-managed / not in the modal chain stack), but .customChange now calls showModal("CustomText"). This will open the Solid modal without hiding/chaining the mobile config modal, likely leaving two dialogs stacked and breaking the “return to previous modal” behavior. Either keep legacy chaining (show CustomText in the same chain) or migrate mobileTestConfigModal to the modal store so chaining works.

Suggested change
showModal("CustomText");
void modal.hide().then(() => {
showModal("CustomText");
});

Copilot uses AI. Check for mistakes.
const val = parseInt(input(), 10);

if (val === null || isNaN(val) || val < 0 || !isFinite(val)) {
showNoticeNotification("Custom word amount must be at least 1");
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Validation message doesn't match the actual allowed values: val < 0 is rejected but 0 is explicitly supported as “Infinite words”, yet the notice says “must be at least 1”. Update the message (and/or validation) to reflect the intended range (e.g., “0 or greater”).

Suggested change
showNoticeNotification("Custom word amount must be at least 1");
showNoticeNotification("Custom word amount must be 0 or greater");

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +19
import { createSignal } from "solid-js";

export type CustomTextIncomingData = {
text: string;
set?: boolean;
long?: boolean;
} | null;

const [customTextIncomingData, setCustomTextIncomingData] =
createSignal<CustomTextIncomingData>(null);

const [textToSave, setTextToSave] = createSignal<string[]>([]);

export {
customTextIncomingData,
setCustomTextIncomingData,
textToSave,
setTextToSave,
};
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

This state module appears unused: there are no imports/references to frontend/src/ts/states/custom-text-modal.ts in the codebase, and CustomTextModal.tsx manages its own local signals instead. Consider removing this file or switching the modal implementation to consume it to avoid dead/duplicated state.

Copilot uses AI. Check for mistakes.
</div>
<div>
<mount data-component="testconfig"></mount>
<div id="testConfig" class="full-width">
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

<mount data-component="testconfig"> adds the new Solid TestConfig, but the legacy #testConfig markup is still rendered right below it. On ≥sm screens the Solid component is visible (sm:grid) and the legacy #testConfig is also visible, so users will see (and interact with) two different test config UIs at once. Either remove the legacy #testConfig block or hide it when the Solid mount is present (and update/remove the legacy event handlers accordingly).

Suggested change
<div id="testConfig" class="full-width">
<div id="testConfig" class="full-width sm:hidden">

Copilot uses AI. Check for mistakes.
Comment on lines +434 to +436
onClick={() => {
showErrorNotification("//todo");
}}
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

The quote search button currently triggers showErrorNotification("//todo"), which is an unhelpful user-facing error and leaves the feature non-functional. Wire this to the existing quote search flow (e.g., open the quote search modal) or remove/disable the button until implemented.

Suggested change
onClick={() => {
showErrorNotification("//todo");
}}
disabled

Copilot uses AI. Check for mistakes.
@socket-security
Copy link

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm vite is 91.0% likely obfuscated

Confidence: 0.91

Location: Package overview

From: pnpm-lock.yamlnpm/vite@8.0.1

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/vite@8.0.1. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@Miodec Miodec marked this pull request as draft March 23, 2026 16:05
@github-actions github-actions bot removed the waiting for review Pull requests that require a review before continuing label Mar 23, 2026
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants