Skip to content

Fix: theme correction in iframe and remove tabs bar dropdown#1102

Open
ShinichiShi wants to merge 1 commit into
CircuitVerse:mainfrom
ShinichiShi:fix-embed-view-bugs
Open

Fix: theme correction in iframe and remove tabs bar dropdown#1102
ShinichiShi wants to merge 1 commit into
CircuitVerse:mainfrom
ShinichiShi:fix-embed-view-bugs

Conversation

@ShinichiShi

@ShinichiShi ShinichiShi commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #1101

Describe the changes you have made in this PR -

This PR fixes both the issue and also adds some improvements to the URL parsing

Screenshots of the UI changes (If any) -


Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

  • Embed URL query params (?theme=, ?clock_time=, etc.) were read from route.query which is empty at mount time when Embed is rendered via v-if (not ). Fixed with URLSearchParams fallback.

  • Theme not applying: updateThemeForStyle(undefined) returned early silently as it used to be undefined. Added null guard.

  • Tabs bar chevron toggle (collapse/expand) was visible in embed, which is now conditional, only seen when it is not embed view


Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

Summary by CodeRabbit

  • Refactor

    • Streamlined embed parameter handling by consolidating configuration source reading for better consistency in the embed initialization process.
    • Optimized theme initialization timing and application for improved embed functionality.
  • Style

    • Updated styling on the height-toggle button component in tabs.

@netlify

netlify Bot commented Jun 15, 2026

Copy link
Copy Markdown

Deploy Preview for circuitverse ready!

Name Link
🔨 Latest commit 8c20d73
🔍 Latest deploy log https://app.netlify.com/projects/circuitverse/deploys/6a307b08a5898000085af7bb
😎 Deploy Preview https://deploy-preview-1102--circuitverse.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 46 (🟢 up 1 from production)
Accessibility: 70 (no change from production)
Best Practices: 92 (no change from production)
SEO: 82 (no change from production)
PWA: -
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The PR addresses two bugs from issue #1101. In TabsBar.vue, the non-embedded height-toggle button gains the tabsbar-toggle CSS class so existing scoped style rules take effect. In embed.vue, query-string preference reading is unified behind a queryParam() helper that checks route.query first and falls back to window.location.search. The computed embed flags (theme, hasDisplayTitle, hasClockTime, hasFullscreen, hasZoomInOut) are updated to use this helper with consistent boolean string comparisons. onBeforeMount now only sets window.logixProjectId when absent or invalid. Theme application in onMounted is made conditional on a valid THEME mapping entry.

Possibly related PRs

  • CircuitVerse/cv-frontend-vue#312: Directly modifies src/pages/embed.vue query-derived computed flags and updateThemeForStyle invocation, overlapping with this PR's embed preference refactor.
  • CircuitVerse/cv-frontend-vue#1082: Adjusts embed-mode initialization in src/globalVariables.ts for window.embed and window.logixProjectId defaults, intersecting with this PR's onBeforeMount guard on window.logixProjectId.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix: theme correction in iframe and remove tabs bar dropdown' directly aligns with the main changes: applying theme fixes to the embed iframe and removing the tabs bar dropdown visibility in embed mode.
Linked Issues check ✅ Passed The PR implements all coding objectives from issue #1101: tabs bar dropdown is now conditional (hidden in embed), and theme application is fixed through URL parameter handling and null guard in updateThemeForStyle().
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue #1101: TabsBar.vue adds CSS class for conditional visibility, and embed.vue refactors parameter reading and theme application logic as required.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/pages/embed.vue (1)

196-198: 💤 Low value

Consider narrowing the type for array query params.

route.query[key] can be string | string[] | undefined, but the function casts directly to string. If a query param appears multiple times (e.g., ?theme=a&theme=b), the value would be an array at runtime, causing string comparisons to silently fail.

For embed URLs this is unlikely since they're programmatically constructed, but a defensive fix would handle it:

Suggested improvement
 function queryParam(key: string): string | null {
-    return (route.query[key] as string) || urlParams.get(key) || null
+    const val = route.query[key]
+    const routeVal = Array.isArray(val) ? val[0] : val
+    return routeVal || urlParams.get(key) || null
 }

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3f573a78-6aea-4c08-ace9-0ca90828043f

📥 Commits

Reviewing files that changed from the base of the PR and between 2514f15 and 8c20d73.

📒 Files selected for processing (2)
  • src/components/TabsBar/TabsBar.vue
  • src/pages/embed.vue

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.

🐞 Bug: [vuesim] Theme selection isn't reflected in the iframe and tabbar remove in embed

1 participant