Skip to content

refactor: replace Fomantic search module with first-party code#37443

Merged
silverwind merged 23 commits into
go-gitea:mainfrom
silverwind:search
May 11, 2026
Merged

refactor: replace Fomantic search module with first-party code#37443
silverwind merged 23 commits into
go-gitea:mainfrom
silverwind:search

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 26, 2026

  • Replace fomantic search code with minimal first-party code
  • Added a small fix to vertically align search box and search button
  • Manually tested all search forms.
  • Add errorName helper, similar to errorMessage.
image

This PR was written with the help of Claude Opus 4.7

Drops the 1565-line vendored Fomantic UI search jQuery plugin in favor
of a small first-party TypeScript module covering the three call sites
(repo, user, and team autocomplete). Adds an e2e regression suite that
exercises each search box.

Also fixes input/button vertical alignment in the four forms that wrap
a search box: the fomantic-era `tw-align-middle` workaround is replaced
by `flex-text-block` on the form, which is the codebase's standard
flex helper for this layout.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 26, 2026
@silverwind silverwind added type/refactoring Existing code has been cleaned up. There should be no new functionality. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Apr 26, 2026
Three boxes (repo/user/team) all share the same first-party module, so
one happy-path test on the user box is sufficient as a regression
gate. Drops the org/team helpers in tests/e2e/utils.ts that only those
extra tests motivated.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind marked this pull request as draft April 26, 2026 18:53
silverwind and others added 3 commits April 26, 2026 21:05
Move web_src/js/modules/fomantic/search.ts to web_src/js/modules/search.ts
(no Fomantic dependency anymore). Replace the imperative
initSearchBox(el, opts) with chooseFromApi(el, url, parse) that returns
a Promise<SearchResult> resolving with the user's chosen item; callers
loop with `while (box.isConnected) { const pick = await ...; ... }` and
own writing back to whatever input/state they manage.

Cleanup is via a single AbortController whose signal is passed to all
listeners; the in-flight fetch has its own controller so a new keystroke
can cancel just the request without tearing down listeners.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind changed the title Replace Fomantic search module with first-party autocomplete Replace Fomantic search module with first-party code Apr 26, 2026
- chooseFromApi is now generic over the response shape; each caller
  declares a typed *SearchResponse, removing four `any` annotations
- e2e test prefix: rc- → repo-collab- for legibility in the test DB
- web_src/css/modules/search.css: drop the stale leading comment

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind marked this pull request as ready for review April 26, 2026 19:16
@bircni bircni self-requested a review April 26, 2026 19:19
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 26, 2026
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

AI slop, do you agree?

Why you force every caller to write a while loop?

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 26, 2026
@silverwind
Copy link
Copy Markdown
Member Author

You mean the callback on chooseFromApi? It's not ideal API I agree.

@wxiaoguang
Copy link
Copy Markdown
Contributor

You mean the callback on chooseFromApi? It's not ideal API I agree.

Why a framework "component" forces the caller to handle the value update and event dispatch by themselves and copy & paste the code again and again?

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 26, 2026

The main problem is that every consumer needs a different response, but the fetching logic is shared. I guess I can just deliver a generic response to each consumer.

@wxiaoguang
Copy link
Copy Markdown
Contributor

That's why Gitea's frontend was totally a mess, I had to spend plenty of time to refactor almost every line.

silverwind and others added 2 commits April 27, 2026 00:44
- attachSearchBox(el, url, parse) replaces the await-style chooseFromApi
- The component handles input update + change-event dispatch on
  selection, so callers don't loop or write back the value themselves
- Each caller is now a single statement; response shape is conveyed via
  the parse callback's parameter annotation (T inferred)

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
After clicking the search result, the test now also clicks Add
Collaborator and asserts the username appears in the page body
(i.e., in the collaborator list after the redirect), proving the
chosen result drives the form submission end-to-end.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Comment thread tests/e2e/repo-collab.test.ts Outdated
Signed-off-by: silverwind <me@silverwind.io>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 26, 2026

Now using callback, similar to what was with fomantic. The while was a misguided attempt at making it use promise, but that obviously can't work in this case.

- collapse the render/keydown/select helpers (replaceChildren with mapped
  array, inline ArrowUp/Down branch, drop the move() helper)
- abort the in-flight fetch from hide() so Escape/blur/outside-click
  cancel the request, and gate the post-await render on
  ctrl.signal.aborted to suppress responses that settle after hide()
- rename idx → index
- restore the leading comment in web_src/css/modules/search.css that an
  earlier commit on this branch dropped

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind requested a review from Copilot April 26, 2026 23:57
silverwind and others added 2 commits May 4, 2026 22:29
Mirrors errorMessage in web_src/js/modules/errors.ts. Migrates the only
two existing `(err as Error).name !== 'AbortError'` sites
(modules/search.ts, features/common-fetch-action.ts).

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@wxiaoguang wxiaoguang dismissed their stale review May 4, 2026 22:25

dismiss

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels May 4, 2026
@silverwind silverwind changed the title Replace Fomantic search module with first-party code refactor: replace Fomantic search module with first-party code May 7, 2026
@lunny
Copy link
Copy Markdown
Member

lunny commented May 7, 2026

It seems the search box in team member is broken.
image

@wxiaoguang wxiaoguang removed their request for review May 9, 2026 09:33
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented May 10, 2026

It seems the search box in team member is broken. image

Can not reproduce on /org/<org>/teams/<team>

image

Regression coverage for the team-members page (org/team/members.tmpl),
whose DOM wraps the input in an extra div.ui.input and was the source
of the user-visible report on PR go-gitea#37443. Adds apiCreateOrg and
apiCreateTeam helpers to tests/e2e/utils.ts.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

@lunny 7388128 adds your case as an e2e test, proving it works.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
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

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

Comment thread web_src/js/modules/search.ts Outdated
Comment thread web_src/js/modules/search.ts Outdated
- search is no longer typed as nullable; the closure's hide() no longer
  references search, so `const search = debounce(...)` is reachable
  after `const hide`. The two dismiss paths (Escape, outside-click)
  cancel the debounce explicitly via a local `dismiss` helper, and the
  blur handler keeps its own inline cancel+deferred-hide.
- The per-call `document.addEventListener('click', ...)` is replaced by
  a single module-level delegated handler that iterates a registry of
  {container, dismiss} entries. Avoids accumulating one handler per
  attached box.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 11, 2026
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 11, 2026
@silverwind silverwind enabled auto-merge (squash) May 11, 2026 04:58
@silverwind silverwind merged commit 5dc9d62 into go-gitea:main May 11, 2026
21 checks passed
@silverwind silverwind deleted the search branch May 11, 2026 05:25
@GiteaBot GiteaBot added this to the 1.27.0 milestone May 11, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 11, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 12, 2026
* main:
  fix(deps): update dependency mermaid to v11.15.0 [security], add e2e test (go-gitea#37662)
  ci: Also lint json5 files (go-gitea#37659)
  fix(templates): avoid misleading compare message when branches lack merge base (go-gitea#37651)
  fix(deps): update npm dependencies (go-gitea#37647)
  refactor: routing info middleware (go-gitea#37653)
  chore(deps): update action dependencies (major) (go-gitea#37638)
  fix(deps): update go dependencies (major) (go-gitea#37639)
  ci(renovate): update Go import paths on major bumps (go-gitea#37641)
  fix(packages): Add label for private and internal package and fix composor package source permission check (go-gitea#37610)
  refactor: replace Fomantic search module with first-party code (go-gitea#37443)
  fix(deps): update npm dependencies (go-gitea#37636)
  fix(deps): update module code.gitea.io/sdk/gitea to v0.25.0 (go-gitea#37637)
  feat(api): add last_sync to repository API (go-gitea#37566)
  test(e2e): run playwright via container (go-gitea#37300)
  feat(editor): broaden language detection in web code editor (go-gitea#37619)
  refactor(log): replace log.Critical with log.Error (go-gitea#37624)
  fix: "run as root" check (go-gitea#37622)
  fix: improve actions status icons and texts (go-gitea#37206)

# Conflicts:
#	pnpm-workspace.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants