Mobile-first index.html rewrite, add smoke-test script and run in Pages workflow#12
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughPR này thay thế toàn bộ giao diện index.html từ mô hình dashboard sang mobile-first, thêm script kiểm thử tự động để xác thực cấu trúc HTML/JavaScript mới, tích hợp smoke test vào CI/CD workflow, và cập nhật tài liệu hướng dẫn. Cây cấu trúc, CSS styling, JavaScript behavior được thiết kế lại hoàn toàn; tài liệu và lỗi nhỏ cũng được sửa. ChangesMobile-First UI Redesign và Smoke Test Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5868608f5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/static.yml:
- Around line 25-27: The workflow step named "Smoke test static app" runs node
scripts/smoke-test-static.mjs without pinning Node; add an actions/setup-node
step before that step to install a specific Node version (use node-version: 20
to match other workflows) so scripts/smoke-test-static.mjs runs with a
consistent runtime; ensure the setup step appears before the "Smoke test static
app" run step and uses actions/setup-node@v3 (or current stable) with
node-version: 20.
In `@index.html`:
- Around line 219-221: The three non-submit buttons (the element with id
"mobileMenu", the element with class "login-button", and the other icon button
with class "icon-button") are missing explicit type attributes and default to
"submit"; add type="button" to each of these button elements so they do not
trigger form submission if ever placed inside a form.
- Around line 262-264: The routing currently disables deep-linking/back-forward
because setRoute (used in bind on click) does not update location.hash and init
immediately calls history.replaceState(location.pathname) which clears the hash;
fix by making setRoute update location.hash to the route id (e.g., location.hash
= '#'+id) instead of only manipulating DOM, adjust the click handler in bind to
let hashchange drive view changes (call setRoute only from the hashchange
listener or avoid calling setRoute directly after setting location.hash), and
remove or relocate the history.replaceState(...) that erases the hash in init so
you preserve hashes for navigation and deep links; refer to the functions
setRoute, bind, init and the history.replaceState call when making changes.
- Line 264: The init function relies on implicit global DOM variables
bottomNav/sideNav/drawerNav (via element IDs) which is fragile; update init to
explicitly query and cache those elements (e.g., const bottomNav =
document.getElementById('bottomNav') or document.querySelector(...)) before
using bottomNav.innerHTML/sideNav.innerHTML/drawerNav.innerHTML and any later
references (renderHome/renderAI/renderWorkspace/renderCode/renderProfile/bind
may assume they exist), and guard against null by throwing or returning early if
any element is not found.
In `@scripts/smoke-test-static.mjs`:
- Around line 30-39: Hiện đoạn tạo và kiểm tra script tạm (scripts.length,
mkdtempSync -> workDir, scriptPath, writeFileSync, spawnSync) có thể throw trước
khi rmSync được gọi; sửa bằng cách bọc tạo/ghi và spawnSync vào một khối try and
đặt rmSync(workDir, { recursive: true, force: true }) trong finally để luôn
cleanup thư mục tạm even khi có exception.
- Around line 24-25: The current smoke test assertions in
scripts/smoke-test-static.mjs only check for a specific literal
`<p>${m.text}</p>` which is too narrow; update the test that uses the
variables/html string (the assertions checking html.includes(...)) to detect any
unescaped interpolation of m.text into innerHTML by replacing the single-pattern
check with a broader check (e.g., use a regex or string search against patterns
like `${m.text}` appearing inside HTML tags or assignments to innerHTML) and
keep the existing assert that escaped content (esc(m.text)) is present; target
the two assertions around html/includes to ensure any direct `${m.text}`
interpolation into rendered HTML is rejected, not just the `<p>` case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80810a30-292e-48bb-a338-041a087a1e24
📒 Files selected for processing (5)
.github/workflows/hadolint.yml.github/workflows/static.ymlREADME.mdindex.htmlscripts/smoke-test-static.mjs
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/static.yml">
<violation number="1" location=".github/workflows/static.yml:26">
P2: The workflow runs `node scripts/smoke-test-static.mjs` without a `setup-node` step, relying on the runner's pre-installed Node version which can change without notice. Add `actions/setup-node@v4` with a pinned `node-version` (e.g., 20) before this step to ensure consistent behavior, consistent with other workflows in this repo.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Signed-off-by: Huỳnh Thương <252359928+Huynhthuongg@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Signed-off-by: Huỳnh Thương <252359928+Huynhthuongg@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Signed-off-by: Huỳnh Thương <252359928+Huynhthuongg@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Signed-off-by: Huỳnh Thương <252359928+Huynhthuongg@users.noreply.github.com>
Motivation
Description
index.htmlto a mobile-first layout: changedlangtovi, updated metadata/title, replaced desktop layout with a compactmobile-shell, added inline SVG icons, accessible attributes, and UI/UX improvements while ensuring chat messages are escaped before insertion viaesc(...).scripts/smoke-test-static.mjswhich asserts presence of required route containers, mobile layout overflow rules, bottom nav tap targets, chat input/send controls, escaping of chat messages, and runs anode --checksyntax validation on inline scripts..github/workflows/static.ymlto run the smoke test stepnode scripts/smoke-test-static.mjsduring thebuildjob before preparing and uploading the_siteartifact.README.mdto reference the smoke test and added a small example project tree entry for the new script, and fixed a typo in.github/workflows/hadolint.ymlcomment.Testing
node scripts/smoke-test-static.mjs, and the script completed successfully (no failures reported).--checkmode and reported no syntax errors.Codex Task
Summary by cubic
Rewrote the static app into a mobile-first, Vietnamese single-file UI and added a smoke test to block regressions before GitHub Pages deploys. Improves mobile UX, ensures escaped chat rendering, and tightens CI with workflow fixes, including the SLSA release trigger and output.
New Features
index.html:lang="vi", updated metadata, compact mobile shell, enlarged fixed bottom nav (64px tap targets, safe-area insets), inline SVG icons, and a11y tweaks.esc(...).scripts/smoke-test-static.mjs: asserts required routes, prevents horizontal overflow, verifies fixed bottom nav/tap targets, requires chat input/send, forbids rawinnerHTML, and runs Node--checkon inline scripts..github/workflows/static.ymlruns the smoke test before building_site; README documents the test.Bug Fixes
actions/first-interactionto v1.3.0 commit SHA ingreetings.yml.google.ymlby using branchmain.publishedand referencesteps.hash.outputs.hashesin outputs.Written for commit 55839d2. Summary will update on new commits.