refactor: update provider panels for improved layout and styling#7248
refactor: update provider panels for improved layout and styling#7248
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
ProviderModelsPanel.vue, the.provider-compact-itemstyle is defined but never applied in the template, so it can either be removed or wired up to the relevant list items to avoid dead CSS. - The
max-height: calc(100vh - 335px);on.provider-source-liststill relies on a hard-coded magic offset even though the header/layout have been refactored; consider tying this to a more robust layout mechanism (e.g., flex or CSS grid) or recomputing the offset so it accurately reflects the new header/footer sizes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ProviderModelsPanel.vue`, the `.provider-compact-item` style is defined but never applied in the template, so it can either be removed or wired up to the relevant list items to avoid dead CSS.
- The `max-height: calc(100vh - 335px);` on `.provider-source-list` still relies on a hard-coded magic offset even though the header/layout have been refactored; consider tying this to a more robust layout mechanism (e.g., flex or CSS grid) or recomputing the offset so it accurately reflects the new header/footer sizes.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/ConversationPage.vue" line_range="99-108" />
<code_context>
:placeholder="tm('models.searchPlaceholder')"
/>
- <v-spacer></v-spacer>
- <v-btn
- color="primary"
- prepend-icon="mdi-download"
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider adding an accessible label to the inline edit icon button.
Because this is an icon-only button, it needs an accessible label so screen readers announce its purpose. Please add an ARIA label such as `:aria-label="tm('conversations.edit')"` (and optionally `:title="tm('conversations.edit')"`) so both assistive tech and mouse users understand its function.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <v-btn | ||
| icon | ||
| variant="plain" | ||
| size="x-small" | ||
| density="compact" | ||
| :ripple="false" | ||
| class="conversation-inline-edit" | ||
| @click.stop="editConversation(item)" | ||
| :disabled="loading" | ||
| > |
There was a problem hiding this comment.
issue (bug_risk): Consider adding an accessible label to the inline edit icon button.
Because this is an icon-only button, it needs an accessible label so screen readers announce its purpose. Please add an ARIA label such as :aria-label="tm('conversations.edit')" (and optionally :title="tm('conversations.edit')") so both assistive tech and mouse users understand its function.
There was a problem hiding this comment.
Code Review
This pull request refactors the UI for provider management and conversation lists, including updates to the Material Design Icons subset and improved responsive layouts for the Provider Models and Sources panels. Key functional changes include moving the conversation edit action to an inline button and replacing collapsible advanced configuration panels with static sections. Feedback suggests improving accessibility by adding tooltips to new buttons, refining conditional logic for displaying configuration sections to prevent empty UI elements, and evaluating whether keeping advanced settings collapsible would better maintain a clean interface.
| <v-btn | ||
| icon | ||
| variant="plain" | ||
| size="x-small" | ||
| density="compact" | ||
| :ripple="false" | ||
| class="conversation-inline-edit" | ||
| @click.stop="editConversation(item)" | ||
| :disabled="loading" | ||
| > | ||
| <v-icon size="14">mdi-pencil</v-icon> | ||
| </v-btn> |
There was a problem hiding this comment.
The new inline edit button is quite small and lacks a tooltip. Adding a tooltip would improve accessibility and help users understand the button's purpose, especially since it's placed directly next to the title text.
<v-tooltip location="top">
<template v-slot:activator="{ props }">
<v-btn
v-bind="props"
icon
variant="plain"
size="x-small"
density="compact"
:ripple="false"
class="conversation-inline-edit"
@click.stop="editConversation(item)"
:disabled="loading"
>
<v-icon size="14">mdi-pencil</v-icon>
</v-btn>
</template>
{{ tm('dialogs.edit.title') }}
</v-tooltip>
| @delete-provider="deleteProvider" | ||
| @add-model-provider="addModelProvider" | ||
| /> | ||
| <section v-if="advancedSourceConfig" class="provider-section"> |
There was a problem hiding this comment.
The v-if="advancedSourceConfig" check will always evaluate to true as long as a provider is selected, even if it has no advanced configuration fields (resulting in an empty section title being displayed). It's better to check if the object contains any enumerable keys.
<section v-if="advancedSourceConfig && Object.keys(advancedSourceConfig).length > 0" class="provider-section">
| <section v-if="advancedSourceConfig" class="provider-section"> | ||
| <div class="provider-section-head"> | ||
| <div class="provider-section-title">{{ tm('providerSources.advancedConfig') }}</div> | ||
| </div> | ||
| <AstrBotConfig | ||
| :iterable="advancedSourceConfig" | ||
| :metadata="providerSourceSchema" | ||
| metadataKey="provider" | ||
| :is-editing="true" | ||
| /> | ||
| </section> |
There was a problem hiding this comment.
Replacing the collapsible v-expansion-panels with a static section makes the advanced configuration always visible. For providers with many settings, this can clutter the UI and push the important "Models" section further down. Consider keeping this section collapsible to maintain a cleaner default view.
|
No docs changes were generated in this run (docs repo had no updates). Docs repo: AstrBotDevs/AstrBot-docs AI change summary (not committed):
Experimental bot notice:
|
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
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
Refine the provider management and conversation pages layout and styling for a more structured, modern configuration experience.
Enhancements: