frontend: fix full_page extensions not showing up#3751
frontend: fix full_page extensions not showing up#3751patrickelectric merged 1 commit intobluerobotics:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the BrIframe container positioning so full-page iframe-based extensions render correctly by switching its v-sheet style from relative to absolute positioning. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Changing the iframe container from
position: relativetoposition: absolutecan have broad layout implications; consider scoping this with a dedicated class or only enabling it for full-page extensions to avoid unintentionally affecting other iframe usages. - It would help future readers to add a short comment in the template or component explaining why
position: absoluteis required here for full-page extensions, since it’s not obvious from the markup alone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Changing the iframe container from `position: relative` to `position: absolute` can have broad layout implications; consider scoping this with a dedicated class or only enabling it for full-page extensions to avoid unintentionally affecting other iframe usages.
- It would help future readers to add a short comment in the template or component explaining why `position: absolute` is required here for full-page extensions, since it’s not obvious from the markup alone.
## Individual Comments
### Comment 1
<location> `core/frontend/src/components/utils/BrIframe.vue:5` </location>
<code_context>
width="100%"
height="100%"
- style="overflow: hidden; position: relative;"
+ style="overflow: hidden; position: absolute;"
>
<spinning-logo
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching to absolute positioning may break layout or stacking in some usages of BrIframe.
Because `position: absolute` removes the element from normal flow and depends on its parent’s positioning, this could change sizing, scrolling, or stacking in existing `BrIframe` usages. Please verify all `BrIframe` consumers have suitable parent containers and consider whether we need explicit positioning (`top/left`), `z-index`, or a wrapper to avoid layout regressions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| width="100%" | ||
| height="100%" | ||
| style="overflow: hidden; position: relative;" | ||
| style="overflow: hidden; position: absolute;" |
There was a problem hiding this comment.
issue (bug_risk): Switching to absolute positioning may break layout or stacking in some usages of BrIframe.
Because position: absolute removes the element from normal flow and depends on its parent’s positioning, this could change sizing, scrolling, or stacking in existing BrIframe usages. Please verify all BrIframe consumers have suitable parent containers and consider whether we need explicit positioning (top/left), z-index, or a wrapper to avoid layout regressions.
|
I have the same problem with sonarview with this PR, but cockpit works just fine.. |
maybe your css was cached? I just re-tested and it worked fine on the PR... |
The issue can be seen by trying to open sonarview
I'm not sure if this breaks anything from #3714
It doesn't look like it, for me.
Summary by Sourcery
Bug Fixes: