fix(dashboard): make skill editor dialog responsive on small screens - #9873
Open
mantoujun12 wants to merge 2 commits into
Open
fix(dashboard): make skill editor dialog responsive on small screens#9873mantoujun12 wants to merge 2 commits into
mantoujun12 wants to merge 2 commits into
Conversation
- Enable fullscreen mode for skill editor dialog on mdAndDown breakpoints - Switch skill-editor grid to single-column stacked layout on mobile - Constrain files panel height and enforce monaco editor min-height
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/components/extension/SkillsSection.vue" line_range="646" />
<code_context>
<v-dialog
v-model="editorDialog.show"
max-width="1180px"
+ :fullscreen="$vuetify.display.mdAndDown"
:persistent="editorDialog.saving"
>
</code_context>
<issue_to_address>
**issue (broader_impact):** On short mobile viewports, the fullscreen dialog still contains `.skill-editor-dialog { max-height: min(88vh, 980px); overflow: hidden; }`, while the editor alone has a `min-height` of `40vh` and the header, file panel, and actions require additional height. The card therefore clips the editor or save/cancel actions instead of providing a usable fullscreen layout.
**Triggers:** When the viewport height is small, such as a phone in landscape or a short mobile browser viewport.
**Suggested fix:** Override the card's max-height and overflow for the fullscreen state, or size the dialog body and its grid rows within the available viewport height instead of using independent `vh` minimums.
</issue_to_address>
### Comment 2
<location path="dashboard/src/components/extension/SkillsSection.vue" line_range="646" />
<code_context>
<v-dialog
v-model="editorDialog.show"
max-width="1180px"
+ :fullscreen="$vuetify.display.mdAndDown"
:persistent="editorDialog.saving"
>
</code_context>
<issue_to_address>
**issue:** The dialog switches to fullscreen through Vuetify's `mdAndDown` breakpoint, which is 960px by default, but the single-column editor layout is enabled only at `max-width: 860px`. Between 861px and 959px the dialog is fullscreen while the editor remains a 280px-plus-editor two-column grid, so the narrow layout problem persists at part of the breakpoint range.
**Triggers:** When the viewport width is between 861px and 959px.
**Suggested fix:** Use the same breakpoint as Vuetify's `mdAndDown` behavior, typically `@media (max-width: 960px)`, or change the fullscreen condition to match the CSS breakpoint.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: dashboard/src/components/extension/SkillsSection.vue:646, dashboard/src/components/extension/SkillsSection.vue:646
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Address Sourcery-ai broader_impact finding on PR AstrBotDevs#9873: the base `.skill-editor-dialog { max-height: min(88vh, 980px); overflow: hidden; }` remains active in fullscreen mode on short mobile viewports (e.g. landscape phones), where the stacked 20vh file tree + 40vh editor plus title and action bar can exceed 88vh. Combined with `overflow: hidden`, content gets clipped and the save/cancel buttons become unreachable. Override `.skill-editor-dialog` within the existing `@media (max-width: 860px)` block to lift the 88vh cap and allow vertical scrolling when content overflows the viewport. This is a minimal fix for the temporary solution; the 860-960px gap between the media query and Vuetify's `mdAndDown` (960px) breakpoint is accepted as a known limitation.
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/components/extension/SkillsSection.vue" line_range="646" />
<code_context>
<v-dialog
v-model="editorDialog.show"
max-width="1180px"
+ :fullscreen="$vuetify.display.mdAndDown"
:persistent="editorDialog.saving"
>
</code_context>
<issue_to_address>
**issue (bug_risk):** The dialog enters fullscreen for Vuetify's `mdAndDown` range (up to 960px), but the stacked layout and overflow override apply only at `max-width: 860px`. Between 861px and 960px, the editor keeps its two-column layout and 560px minimum height while `.skill-editor-dialog` retains `max-height: min(88vh, 980px)` and `overflow: hidden`, so on short tablet or landscape viewports the editor content and save/cancel buttons are clipped and cannot be reached.
**Triggers:** When the viewport width is 861–960px and its height is too short for the base editor layout, such as a tablet or landscape window.
**Suggested fix:** Apply the responsive layout and dialog overflow override at the same breakpoint as `mdAndDown`, or remove the fullscreen behavior from the unmatched 861–960px range.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: dashboard/src/components/extension/SkillsSection.vue:646
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
此 Pull Request 为临时解决方案
如果短期内有重置 Skill 编辑器 的计划,可关闭此 PR
如果不需要中等窗口以下的窗口全屏,可追加 commit 移除 646行 的
:fullscreen="$vuetify.display.mdAndDown"有什么改进方案均可在此 PR 追加新 commit 或 comment 。或者关闭此 PR 时,可以说明一下不方便改动的原因
Modifications / 改动点
Fixes #9871
本 Pull Request 仅修改了
SkillsSection.vue并添加了两处内容 (窗口全屏条件CSS):646行添加了:fullscreen="$vuetify.display.mdAndDown",避免和固定在右下角的两个按钮出现元素重叠2405-2441行添加了3个选择器关于3个选择器做了什么
.skill-editor编辑器弹窗内文件树/编辑器在窄屏下改成单列布局.skill-editor__files固定为20vh.skill-editor__monaco固定为40vh如果你认为这个固定数值不合理,可以说明一下原因和相对应的解决方法,或者追加
commit也可以。Screenshots or Test Results / 运行截图或测试结果
Before / 改动前
由于双列布局的原因,在移动端下,文件夹树占比较大,编辑器几乎不可见。
After / 改动后
在移动端下,改为单列布局,现在用户可以看见编辑器且查看和编辑内容了。并且由于窗口全屏,现在不会和固定在右下角的按钮产生元素重叠。
对于桌面端,小窗口和中等窗口也有效果,但影响不大。
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Improve the skill editor’s responsive behavior so users can view and edit files reliably on small screens.
Bug Fixes:
Enhancements: