fix: Optimize tiny-engine official website UI#79
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
WalkthroughReplaced hardcoded help overlay image path with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppComponent as ApplicationVisit
participant Loader as Loading.service
participant IFrame as iframe
User->>AppComponent: navigates to visit page
AppComponent->>Loader: start loading (target `#loading`)
AppComponent->>IFrame: set src / insert iframe (hidden)
IFrame-->>AppComponent: load event fires
AppComponent->>Loader: close loading
AppComponent->>IFrame: show iframe
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/tiny-engine-portal/src/application/ApplicationVisit.vue`:
- Around line 2-5: The iframe is not hidden initially so the loading overlay
doesn't mask it; update the initial state by hiding the iframe (element with
class tiny-engine-iframe) until its onload handler runs — either set its inline
style to display: none in the template or add a CSS rule for .tiny-engine-iframe
{ display: none; } and let the existing onload handler (the function that
currently sets display: 'block') reveal it; ensure the loading element (id
"loading") stays visible until the iframe is shown.
- Around line 36-44: The code creates a Loading.service instance assigned to
loadingInstance.value but never closes it; change the iframe onload handler to
call loadingInstance.value.close() (instead of manually hiding the DOM element)
and then set the iframe display if needed. Locate the Loading.service usage
(loadingInstance) and the iframe onload assignment
(document.querySelector('iframe').onload) and replace the DOM hide call with a
call to loadingInstance.value.close() to properly clean up the service instance.
🧹 Nitpick comments (2)
packages/tiny-engine-portal/src/application/ApplicationVisit.vue (2)
33-34: Use a template ref instead ofdocument.querySelector('iframe').Multiple bare
querySelector('iframe')calls are fragile — they'll match the first iframe in the document, which may not be this one if the page structure changes. A Vue template ref (ref="iframeRef") is safer and idiomatic.Proposed refactor
In the template:
- <iframe class="tiny-engine-iframe" :src="tinyEngineUrl"></iframe> + <iframe ref="iframeRef" class="tiny-engine-iframe" :src="tinyEngineUrl" style="display: none"></iframe>In setup:
+ const iframeRef = ref(null)In
onMounted:- const iframe = document.querySelector('iframe') - iframe.src = `${TINY_ENGINE_DESIGNER_URL}?${type}` + iframeRef.value.src = `${TINY_ENGINE_DESIGNER_URL}?${type}` ... - document.querySelector('iframe').onload = function () { - document.querySelector('iframe').style.display = 'block' + iframeRef.value.onload = function () { + iframeRef.value.style.display = 'block'And return
iframeReffrom setup.Also applies to: 41-43
29-35: Inconsistent indentation insideonMounted.Lines 32–35 have an extra level of indentation compared to lines 30–31, though they're at the same logical scope.
Summary by CodeRabbit
New Features
Chores
Style