refactor(modals): remove jquery in ts/modals (@fcasibu) #7292

fcasibu · 2025-12-25T10:05:53Z

Description

Refactor frontend/ts/modals/* to replace jQuery (some still TODO). I've also modified dom.ts to add setValue for ElementsWithUtils<ElementWithValue> instance. Still trying to grep the whole flow/codebase, so usage of qsr or qs is uncertain (Probably use qsr always? Can find non-existing elements that way or developer error). There are a lot of atomic commits in this PR, feel free to squash.

While doing the refactor, also stumbled upon things that I have questions for (didn't made any changes to them), which I thought can be improved:

custom-generator.ts:146: The user is able to write minLength > maxLength. Is this behavior correct?
custom-test-duration.ts:99: It seems like parseInput, always expects a non nullable string, and we seem to always expect that #customTestDurationModal input to always exist (so I used qsr), which makes the conditions val !== null, !isNaN(val) seem to be unnecessary (val is never null or NaN, tbh also isFinite, and val >= 0 is the only valid condition)
custom-text.ts:169: These elements does not seem exist .randomWordsCheckbox, .replaceNewlineWithSpace, .typographyCheck, and .delimiterCheck (last two is just in a challenge file). Are they good to remove?
edit-preset.ts:146: Noticed that we're not updaitng the DOM, since for the most part, in the logic, we mostly use state.checkboxes, so no problem happens, but I think it is also a good idea to update the DOM to match current state? Either calling updateUI() here or just changing the checked value inline.
quote-rate.ts:89: getQuoteStats expects a quote but we're not really passing anything to it, so this essentially does nothing, since it returns immediately on !quote

Checks

Adding/modifying Typescript code?
- I have used qs, qsa or qsr instead of JQuery selectors.
Check if any open issues are related to this PR; if so, be sure to tag them below.
Make sure the PR title follows the Conventional Commits standard. (https://www.conventionalcommits.org for more info)
Make sure to include your GitHub username prefixed with @ inside parentheses at the end of the PR title.

Related #7186

#7186

fcasibu · 2025-12-25T15:09:51Z

frontend/src/ts/modals/edit-preset.ts

+  const action = qsr("#editPresetModal .modal").getAttribute("data-action");
+  const propPresetName = qsa<HTMLInputElement>(
+    "#editPresetModal .modal input",
+  )[0]?.getValue() as string;


Can likely just query for a specific selector, but uncertain. Same for line 237

Yeah, query the specific one instead.

Miodec

In files that use AnimatedModal, (for example in edit-preset) instead of constantly querying for "#editPresetModal .modal, i think i would prefer using modal.getModal().

So:
qsr("#editPresetModal .modal").setAttribute("data-action", "edit");
becomes:
modal.getModal().setAttribute("data-action", "edit");

Copilot

Pull request overview

This PR refactors modal files in frontend/ts/modals/* to remove jQuery dependencies and replace them with custom DOM utility methods. The changes add a setValue method for ElementsWithUtils<ElementWithValue> instances and modify toggleClass to accept an optional force parameter.

Key changes:

Replaced jQuery selectors ($()) with custom DOM utilities (qs, qsr, qsa)
Replaced jQuery methods like .val(), .prop(), .attr() with custom wrappers (.getValue(), .setValue(), .isChecked(), .setAttribute(), etc.)
Added event delegation support using .onChild() for dynamically generated content

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 21 comments.

Show a summary per file

File	Description
frontend/src/ts/utils/dom.ts	Added `setValue` method to `ElementsWithUtils` class and optional `force` parameter to `toggleClass`
frontend/src/ts/modals/word-filter.ts	Removed jQuery selectors and methods, replaced with custom DOM utilities
frontend/src/ts/modals/version-history.ts	Replaced jQuery with `qsr()` for modal content manipulation
frontend/src/ts/modals/user-report.ts	Converted jQuery selectors to `qsr()` for form input retrieval
frontend/src/ts/modals/simple-modals.ts	Refactored event handlers from jQuery delegation to `.onChild()` pattern
frontend/src/ts/modals/share-test-settings.ts	Replaced jQuery with custom DOM utilities for checkbox and input handling
frontend/src/ts/modals/share-custom-theme.ts	Converted jQuery color input retrieval to `qsr().getValue()`
frontend/src/ts/modals/saved-texts.ts	Replaced jQuery with `qs`, `qsr` for list manipulation and event handling
frontend/src/ts/modals/save-custom-text.ts	Converted checkbox manipulation from jQuery to custom utilities
frontend/src/ts/modals/quote-submit.ts	Replaced jQuery form input retrieval with `qsr().getValue()`
frontend/src/ts/modals/quote-search.ts	Refactored dropdown handling and result list manipulation
frontend/src/ts/modals/quote-report.ts	Converted form input retrieval to custom DOM utilities
frontend/src/ts/modals/quote-rate.ts	Replaced jQuery text/star manipulation with `qsr().setText()`
frontend/src/ts/modals/quote-approve.ts	Major refactor using `createElementWithUtils` and custom event handling
frontend/src/ts/modals/practise-words.ts	Converted button state management to custom utilities
frontend/src/ts/modals/mobile-test-config.ts	Replaced jQuery mode/config button handling
frontend/src/ts/modals/last-signed-out-result.ts	Converted result display to custom DOM utilities
frontend/src/ts/modals/google-sign-up.ts	Replaced jQuery input retrieval and button state management
frontend/src/ts/modals/edit-result-tags.ts	Converted tag button iteration to `qsa()`
frontend/src/ts/modals/edit-profile.ts	Refactored badge selection event handling
frontend/src/ts/modals/edit-preset.ts	Complex refactor of preset checkbox and input management
frontend/src/ts/modals/dev-options.ts	Replaced jQuery `.prepend()` with `.prependHtml()`
frontend/src/ts/modals/custom-text.ts	Major refactor of custom text UI state management
frontend/src/ts/modals/custom-test-duration.ts	Converted input parsing to custom utilities
frontend/src/ts/modals/custom-generator.ts	Replaced jQuery input retrieval with `qs().getValue()`

Comments suppressed due to low confidence (1)

frontend/src/ts/modals/saved-texts.ts:109

Event delegation issue. Same problem - event handlers for dynamically generated content need event delegation.

  qs("#savedTextsModal .list .savedText .button.name")?.on("click", (e) => {
    const name = (e.target as HTMLElement).textContent;
    CustomTextState.setCustomTextName(name, false);
    const text = getSavedText(name, false);
    hide({ modalChainData: { text, long: false } });
  });

  qs("#savedTextsModal .listLong .savedLongText .button.name")?.on(
    "click",
    (e) => {
      const name = (e.target as HTMLElement)?.textContent;
      CustomTextState.setCustomTextName(name, true);
      const text = getSavedText(name, true);
      hide({ modalChainData: { text, long: true } });
    },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T14:41:47Z

frontend/src/ts/modals/word-filter.ts

-  $("wordFilterModal .presetInput").empty();
+  qsr("#wordFilterModal .languageInput").empty();
+  qsr("#wordFilterModal .layoutInput").empty();
+  qsr("wordFilterModal .presetInput").empty();


Missing '#' selector prefix. The selector should be "#wordFilterModal .presetInput" instead of "wordFilterModal .presetInput".

Suggested change

qsr("wordFilterModal .presetInput").empty();

qsr("#wordFilterModal .presetInput").empty();

Copilot · 2026-01-06T14:41:47Z

frontend/src/ts/modals/word-filter.ts

-  $("#wordFilterModal button.addButton").on("click", () => {
-    $("#wordFilterModal .loadingIndicator").removeClass("hidden");
+  qsr("#wordFilterModal button.addButton").on("click", () => {
+    qsr("#wordFilterModal .loadingIndicator").show();


Inconsistent use of removeClass("hidden") versus show(). Line 315 uses show() while line 139 uses removeClass("hidden"). These should be consistent - either always use show()/hide() or removeClass("hidden")/addClass("hidden").

Copilot · 2026-01-06T14:41:47Z

frontend/src/ts/modals/version-history.ts

+          qsr("#versionHistoryModal .modal").setHtml(
+            `<div class="releases"></div`,
+          );


The setHtml call is missing a closing tag. The HTML string "<div class="releases"></div should be "<div class="releases"></div>" with a closing angle bracket.

Copilot · 2026-01-06T14:41:48Z

frontend/src/ts/modals/version-history.ts

+          qsr("#versionHistoryModal .modal").setHtml(
            `<div class="releases">${msg}</div`,
          );


The setHtml call is missing a closing tag. The HTML string "<div class="releases">${msg}</div should be "<div class="releases">${msg}</div>" with a closing angle bracket.

Copilot · 2026-01-06T14:41:48Z

frontend/src/ts/modals/edit-preset.ts

+  console.log({ updateConfig, propPresetName });
+


Debugging console.log statement should be removed before merging to production.

Suggested change

console.log({ updateConfig, propPresetName });

Copilot · 2026-01-06T14:41:50Z

frontend/src/ts/modals/saved-texts.ts

-  $("#savedTextsModal .list .savedText .button.name").on("click", (e) => {
-    const name = $(e.target).text();
+  qs("#savedTextsModal .list .savedText .button.name")?.on("click", (e) => {
+    const name = (e.target as HTMLElement).textContent;


Potential issue with textContent returning null. The textContent property can return null if the element has no text. This should be handled with a null check or use a fallback value to prevent passing null to setCustomTextName.

Copilot · 2026-01-06T14:41:51Z

frontend/src/ts/modals/saved-texts.ts

    "click",
    (e) => {
-      const name = $(e.target).text();
+      const name = (e.target as HTMLElement)?.textContent;


Potential issue with textContent returning null. The textContent property can return null if the element has no text. This should be handled with a null check or use a fallback value to prevent passing null to setCustomTextName.

Copilot · 2026-01-06T14:41:51Z

frontend/src/ts/modals/edit-preset.ts

+    if (state.checkboxes.get(settingGroup)) {
+      qsr(
+        `#editPresetModal .modal .checkboxList .checkboxTitlePair[data-id="${settingGroup}"] input`,
+      ).setAttribute("checked", "true");
+    } else {
+      qsr(
+        `#editPresetModal .modal .checkboxList .checkboxTitlePair[data-id="${settingGroup}"] input`,
+      ).removeAttribute("checked");
+    }


Incorrect implementation for setting checkbox checked state. Using setAttribute/removeAttribute for the "checked" attribute only affects the initial state in the HTML, not the actual checked property. This should use a method that sets the checked property (e.g., .setChecked()) or directly manipulate .native.checked to properly control checkbox state.

Copilot · 2026-01-06T14:41:51Z

frontend/src/ts/modals/save-custom-text.ts

-      $("#saveCustomTextModal .isLongText").prop("checked", false);
-      $("#saveCustomTextModal button.save").prop("disabled", true);
+      qsr<HTMLInputElement>("#saveCustomTextModal .textName").setValue("");
+      qsr("#saveCustomTextModal .isLongText").removeAttribute("checked");


Using removeAttribute("checked") to uncheck a checkbox is incorrect. This only removes the HTML attribute but doesn't change the actual checked state of the checkbox. Use a method like .setChecked(false) or directly set .native.checked = false.

Suggested change

qsr("#saveCustomTextModal .isLongText").removeAttribute("checked");

qsr<HTMLInputElement>("#saveCustomTextModal .isLongText").setChecked(false);

Copilot · 2026-01-06T14:41:51Z

frontend/src/ts/modals/custom-text.ts

+    qsa(`${popup} .inputs .group[data-id="limit"] input.sections`)?.show();
+    qsa(`${popup} .inputs .group[data-id="limit"] input.words`)?.hide();
  } else {
-    $(`${popup} .inputs .group[data-id="limit"] input.words`).removeClass(
-      "hidden",
-    );
-    $(`${popup} .inputs .group[data-id="limit"] input.sections`).addClass(
-      "hidden",
-    );
+    qsa(`${popup} .inputs .group[data-id="limit"] input.words`)?.show();
+    qsa(`${popup} .inputs .group[data-id="limit"] input.sections`)?.hide();


Incorrect use of qsa on elements. Lines 83-84 and 86-87 use qsa() which returns multiple elements, but then call show()/hide() directly on the result. Since these selectors likely match multiple elements, this is correct usage. However, the inconsistency with lines 63-64 which use qs() for .group elements should be reviewed - if multiple .section elements exist, qsa should be used consistently.

monkeytypegeorge added the frontend User interface or web stuff label Dec 25, 2025

fcasibu force-pushed the refactor/ts-modals-remove-jquery branch from e92798c to d0997c2 Compare December 25, 2025 12:18

fcasibu commented Dec 25, 2025

View reviewed changes

fcasibu force-pushed the refactor/ts-modals-remove-jquery branch 2 times, most recently from c45226a to 83a24cb Compare December 26, 2025 12:50

fcasibu marked this pull request as ready for review December 26, 2025 12:52

github-actions bot added the waiting for review Pull requests that require a review before continuing label Dec 26, 2025

fcasibu force-pushed the refactor/ts-modals-remove-jquery branch from 83a24cb to e2f2cc7 Compare December 26, 2025 13:19

fcasibu added 22 commits December 30, 2025 15:36

refactor: remove jquery in modals/custom-generator

4efae15

refactor: remove jquery in modals/custom-test-duration

8e83998

feat(dom): add setValue method in ElementsWithUtils

dcd8755

refactor: remove jquery in modals/custom-text

b4550fb

refactor: remove jquery in modals/dev-options

6011955

refactor: remove jquery in modals/edit-preset

326531d

refactor: remove jquery in modals/edit-profile

57f71f9

refactor: remove jquery in modals/edit-result-tags

b4fa61d

refactor: remove jquery in modals/google-sign-up

f4629c7

refactor: remove jquery in modals/last-signout-result

dfd13a4

refactor: remove jquery in modals/mobile-test-config

2e0ee98

refactor: remove jquery in modals/practice-words

3f89b9a

refactor: remove jquery in modals/quote-approve

e0078f3

refactor: remove jquery in modals/quote-rate

f850bd5

feat(dom): add force argument to toggleClass

e3cfa82

refactor: remove jquery in modals/quote-report

9487160

refactor: remove jquery in modals/quote-search

bbe3c24

refactor: remove jquery in modals/quote-submit

1c51556

refactor: remove jquery in modals/save-custom-text

4c9d998

refactor: remove jquery in modals/saved-texts

06521f2

refactor: remove jquery in modals/share-custom-theme

00c2dbc

refactor: remove jquery in modals/share-test-settings

0ce3d5c

fcasibu added 4 commits December 30, 2025 15:36

refactor: remove jquery in modals/simple-modals

ee11e16

refactor: remove jquery in modals/user-report

2360d5c

refactor: remove jquery in modals/version-history

3f58ffd

refactor: remove jquery in modals/word-filter

937446c

fcasibu force-pushed the refactor/ts-modals-remove-jquery branch from e2f2cc7 to 937446c Compare December 30, 2025 07:36

Miodec added 4 commits January 5, 2026 15:19

Merge branch 'master' into pr/fcasibu/7292

d3924c0

Merge branch 'master' into refactor/ts-modals-remove-jquery

48e1140

setchecked

07ef285

focus

11c53d9

Miodec requested a review from Copilot January 6, 2026 14:33

Copilot started reviewing on behalf of Miodec January 6, 2026 14:34 View session

github-actions bot removed the waiting for review Pull requests that require a review before continuing label Jan 6, 2026

Miodec requested changes Jan 6, 2026

View reviewed changes

github-actions bot added the waiting for update Pull requests or issues that require changes/comments before continuing label Jan 6, 2026

Copilot AI reviewed Jan 6, 2026

View reviewed changes

	qsr("wordFilterModal .presetInput").empty();
	qsr("#wordFilterModal .presetInput").empty();

	qsr("#saveCustomTextModal .isLongText").removeAttribute("checked");
	qsr<HTMLInputElement>("#saveCustomTextModal .isLongText").setChecked(false);

Uh oh!

refactor(modals): remove jquery in ts/modals (@fcasibu) #7292

Are you sure you want to change the base?

refactor(modals): remove jquery in ts/modals (@fcasibu) #7292

Conversation

fcasibu commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checks

Uh oh!

fcasibu Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Miodec Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Miodec left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fcasibu commented Dec 25, 2025 •

edited

Loading