Skip to content

feat: add simple ui handler - #42

Merged
raf555 merged 12 commits into
mainfrom
feat/ui
Sep 18, 2026
Merged

raf555 merged 12 commits into
mainfrom
feat/ui

Conversation

@raf555

@raf555 raf555 commented Sep 13, 2026 •

Copy link
Copy Markdown
Owner

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical and moderate issues remain unresolved across Docker packaging, HTML safety, request ordering, and search rendering.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a browser-based KBBI UI at /ui, integrated with Fx and packaged through Docker.

Changes:

  • Registers and serves the UI handler.
  • Adds search, suggestions, random word, and word-of-the-day features.
  • Minifies and copies the UI into the Docker image.
File summaries
File Changes Findings
internal/ui/uifx/fx.go Registers the UI module with Fx. None reported.
internal/ui/controller.go Serves static UI assets at /ui. None reported.
Dockerfile Minifies and copies the UI into the image. Line 5 — critical (2 votes): .dockerignore excludes /assets, causing the COPY to fail.
cmd/kbbi/main.go Loads the UI module. None reported.
assets/view/index.html Implements the dictionary frontend. Lines 457 — critical (3 votes): unescaped output is inserted into innerHTML.
Lines 527/596 — moderate (2 votes): stale in-flight requests can overwrite newer results.
Line 421 — nit (2 votes): search input lacks an accessible name.
Line 596 — moderate (1 vote): forcing raw=true can render blank meanings with the sample asset.
Line 599 — moderate (1 vote): all failures are reported as “word not found.”
Line 654 — nit (1 vote): non-keyboard-accessible anchor controls are used.
Review details

Suppressed comments (4)

assets/view/index.html:598

  • The result area is shared by all of the asynchronous actions, but completions are not versioned or cancelled. A search followed quickly by another search, Random, or Word of the Day can therefore have its older response replace the newer result and push the wrong kata URL. Track a shared action/request token and ignore stale completions.
                const data = await fetchJson(`${API_BASE}/entry/${encodeURIComponent(entry)}?raw=true`);
                renderLemma(data);
                if (updateHistory) updateUrl(data.lemma || entry);

assets/view/index.html:596

  • This forces raw=true, but the checked-in sample asset used by the documented local setup contains definition/usageExamples and no rawDefinition/rawUsageExamples fields. ToKBBI(true) therefore selects empty strings, so normal searches render blank meanings; make the API fall back when raw fields are absent or avoid requesting raw data for those assets.
                const data = await fetchJson(`${API_BASE}/entry/${encodeURIComponent(entry)}?raw=true`);

assets/view/index.html:601

  • Every rejection, including network failures and 5xx responses, is reported as “word not found.” That makes an unavailable API look like a missing dictionary entry; only a 404 should use this message, while transport and server errors need a retry/service-unavailable message.
            } catch (e) {
                showState(`Kata “${escapeHtml(entry)}” tidak ditemukan dalam KBBI.`, true);
                if (updateHistory) updateUrl(entry);

assets/view/index.html:654

  • This interactive element is an <a> without href and is only handled through a mouse click, so it cannot be focused or activated from the keyboard; the related-word chips below use the same pattern. Render these controls as buttons or real links and preserve keyboard activation.
                baseWordNote = `<div class="base-word-note">Kata dasar: <a class="chip-link" data-word="${escapeHtml(entry.baseWord)}">${escapeHtml(entry.baseWord)}</a></div>`;
  • Files reviewed: 5/5 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Dockerfile Outdated
Comment thread view/index.html Outdated
Comment thread view/index.html
Comment thread view/index.html Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Five unresolved review findings remain in view/index.html.

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

view/index.html:602

  • These awaited requests are not sequenced against one another. A slow initial WOTD, random-word, or earlier search response can complete after a newer action and overwrite both the displayed lemma and the URL/history. Use a shared request generation or AbortController and check it before every render and history update.
    view/index.html:605
  • This catch treats every failure as a missing word, including network errors, 500 responses, and malformed responses. During an API outage the UI therefore reports that a valid word is absent, while the status/message produced by fetchJson is discarded; preserve the response status and show a generic load error for non-404 failures.

view/index.html:598

  • hideSuggestions() only removes the current DOM; it does not invalidate a runSuggest request already in flight. If the user submits the same query while that request is pending, the equality check in runSuggest still passes and renderSuggestions reopens the dropdown after the result is rendered. Track a suggestion request generation or abort the request when a search starts (and when suggestions are hidden).
            hideSuggestions();
            searchInput.value = entry;
            showLoading();

view/index.html:492

  • When the query changes, the old suggestion list and activeSuggestIndex/currentSuggestions remain until the new request returns. If a suggestion was active, typing another character and pressing Enter during that window selects the old suggestion instead of the current input. Clear the suggestions (or reset the active selection) on every input event before scheduling the new request.
            if (q.length === 0) {
                hideSuggestions();
                return;
            }
            debounceTimer = setTimeout(() => runSuggest(q), 250);

view/index.html:550

  • The suggestion options are generic <div> elements with mouse handlers and no listbox/option semantics, focusability, or ARIA active-descendant relationship. Although the input handles arrow keys, screen readers cannot discover or announce these choices. Expose a combobox/listbox relationship with option roles and an accessible active state, or use semantic interactive elements.
            suggestList.innerHTML = lemmas.map(l => `<div>${escapeHtml(l)}</div>`).join('');
            Array.from(suggestList.children).forEach((el, i) => {
                el.addEventListener('mousedown', (e) => {
                    e.preventDefault();
                    doSearch(lemmas[i]);
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Moderate UI accessibility and stale-response issues remain unresolved.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

view/index.html:790

  • These generated related-word targets are spans (and the base-word target is an <a> without href), but this handler only adds a mouse click listener. They are therefore not keyboard-focusable or activatable, so users who navigate by keyboard cannot follow base/derived/variant links; render them as real buttons/links or add focusability plus Enter/Space handling.

view/index.html:500

  • This only invalidates suggestion requests. If the initial WOTD (or another view request) is still pending when the user edits the input, viewGen stays unchanged and that response can later overwrite the text the user just typed. Bump viewGen on input changes so stale view responses are ignored.
            hideSuggestions(); // drop stale list/selection immediately, and invalidate any in-flight request

view/index.html:650

  • Unlike doSearch, this action never hides or invalidates the suggestion list. A pending _search response can therefore repopulate stale options after the random result loads, and pressing Enter can submit one of those old suggestions. Clear suggestions before starting this request.
        document.getElementById('btnRandom').addEventListener('click', async () => {
            showLoading();

view/index.html:667

  • loadWotd is called both by the button and by popstate, but it does not clear or invalidate suggestions. A visible or in-flight list for the previous query can remain over the WOTD result and still be selected; clear suggestions before the request starts.
        async function loadWotd(updateHistory = true) {
            showLoading();
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@raf555
raf555 marked this pull request as ready for review September 17, 2026 16:33
@raf555

raf555 commented Sep 18, 2026

Copy link
Copy Markdown
Owner Author

lgtm

@raf555
raf555 merged commit f872e46 into main Sep 18, 2026
4 checks passed
@raf555
raf555 deleted the feat/ui branch September 18, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants