-
Notifications
You must be signed in to change notification settings - Fork 45
fix: adjust ComboBox padding and background for flat style #572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideAdjusts the flat ComboBox visual style by removing extra vertical padding, customizing implicit height, always loading the background, and introducing a dedicated flat background palette with state-dependent visibility and colors. Sequence diagram for flat ComboBox background behavior on state changessequenceDiagram
actor User
participant ComboBox
participant BackgroundItem
participant Loader
participant ButtonPanel
User->>ComboBox: Open view
ComboBox->>BackgroundItem: Initialize implicitWidth and implicitHeight
BackgroundItem->>Loader: Create and set anchors.fill
Loader->>Loader: active = true
Loader->>ButtonPanel: Instantiate floatingComponent
ButtonPanel->>ButtonPanel: Evaluate flat
ButtonPanel->>ButtonPanel: Set color1 = flatBackground
ButtonPanel->>ButtonPanel: Set outsideBorderColor = null
ButtonPanel->>ButtonPanel: Set visible = !flat
User->>ComboBox: Hover flat ComboBox
ComboBox->>ButtonPanel: Update controlState to HoveredState
ButtonPanel->>ButtonPanel: visible = true (flat and hovered)
ButtonPanel->>ButtonPanel: color1 = flatBackground.hovered or hoveredDark
User->>ComboBox: Press flat ComboBox
ComboBox->>ButtonPanel: Update controlState to PressedState
ButtonPanel->>ButtonPanel: visible = true (flat and pressed)
ButtonPanel->>ButtonPanel: color1 = flatBackground.pressed or pressedDark
User->>ComboBox: Release and move cursor away
ComboBox->>ButtonPanel: Update controlState to Normal
ButtonPanel->>ButtonPanel: visible = false when flat and not hovered or pressed
ButtonPanel->>ButtonPanel: color1 = flatBackground.normal or normalDark
Class diagram for updated flat ComboBox stylingclassDiagram
class ComboBox {
bool flat
int padding
int leftPadding
int rightPadding
int topPadding
int bottomPadding
int implicitContentWidth
}
class ComboBoxBackgroundItem {
int implicitWidth
int implicitHeight
}
class BackgroundLoader {
bool active
Component sourceComponent
ComboBox comboBox
}
class FloatingComponent {
}
class ButtonPanel {
ComboBox button
D_Palette color1
color outsideBorderColor
bool visible
D_Palette flatBackground
}
class D_Palette {
color normal_common
color normalDark_common
color hovered_common
color hoveredDark_common
color pressed_common
color pressedDark_common
}
ComboBox "1" o-- "1" ComboBoxBackgroundItem : background
ComboBoxBackgroundItem "1" o-- "1" BackgroundLoader : loader
BackgroundLoader "1" o-- "1" FloatingComponent : sourceComponent
FloatingComponent "1" o-- "1" ButtonPanel : root
ButtonPanel "1" o-- "1" ComboBox : button
ButtonPanel "1" o-- "1" D_Palette : flatBackground
ComboBoxBackgroundItem : implicitWidth = flat ? implicitContentWidth + leftPadding + rightPadding else DS_Style_comboBox_width
ComboBoxBackgroundItem : implicitHeight = flat ? 30 else DS_Style_comboBox_height
ComboBox : topPadding = flat ? 0 else padding
ComboBox : bottomPadding = flat ? 0 else padding
BackgroundLoader : active = true
ButtonPanel : color1 = flat ? flatBackground else DS_Style_button_background1
ButtonPanel : outsideBorderColor = flat ? null else DS_Style_button_outsideBorder
ButtonPanel : visible = !flat || controlStatePressed || controlStateHovered
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- The hardcoded
implicitHeight: control.flat ? 30 : DS.Style.comboBox.heightintroduces a magic number; consider deriving the flat height from the existing style metrics so it stays consistent if theme sizing changes. - Setting the
Loadertoactive: truefor all modes may keep background components alive unnecessarily; if possible, tighten the activation condition (e.g., based on visibility or flat state) to avoid extra work. - The
flatBackgroundpalette is defined inline infloatingComponent; if this palette will be reused across controls, consider extracting it into a shared style/palette definition to avoid duplication and ease future adjustments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hardcoded `implicitHeight: control.flat ? 30 : DS.Style.comboBox.height` introduces a magic number; consider deriving the flat height from the existing style metrics so it stays consistent if theme sizing changes.
- Setting the `Loader` to `active: true` for all modes may keep background components alive unnecessarily; if possible, tighten the activation condition (e.g., based on visibility or flat state) to avoid extra work.
- The `flatBackground` palette is defined inline in `floatingComponent`; if this palette will be reused across controls, consider extracting it into a shared style/palette definition to avoid duplication and ease future adjustments.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Modified topPadding and bottomPadding to be 0 when control.flat is true, removing extra vertical padding in flat mode 2. Changed implicitHeight to 30 for flat ComboBox to provide consistent height 3. Set Loader active to always true to ensure background components are properly loaded 4. Added flatBackground palette with semi-transparent colors for different states (normal, hovered, pressed) in both light and dark themes 5. Made background panel visible only in non-flat mode or when flat ComboBox is in pressed/hovered state 6. Updated color1 property to use flatBackground palette when control is flat These changes ensure that flat ComboBox components have proper visual styling with appropriate padding, height, and background colors that respond to user interaction states while maintaining a minimal appearance. Log: Improved visual styling for flat ComboBox with better padding and interactive states fix: 调整扁平样式ComboBox的内边距和背景 1. 当control.flat为true时,将topPadding和bottomPadding修改为0,移除扁平 模式下的额外垂直内边距 2. 将扁平ComboBox的implicitHeight改为30,以提供一致的高度 3. 将Loader的active设置为始终为true,确保背景组件正确加载 4. 添加了flatBackground调色板,包含适用于浅色和深色主题的不同状态(正 常、悬停、按下)的半透明颜色 5. 使背景面板仅在非扁平模式或扁平ComboBox处于按下/悬停状态时可见 6. 更新color1属性,在控件为扁平时使用flatBackground调色板 这些更改确保扁平ComboBox组件具有适当的视觉样式,包括合适的内边距、高度和 响应交互状态的背景颜色,同时保持简约的外观。 Log: 改进了扁平ComboBox的视觉样式,提供更好的内边距和交互状态 PMS: BUG-304333
deepin pr auto review这段代码主要修改了 ComboBox 组件以支持扁平化(flat)样式,并更新了 FlowStyle 中的颜色定义。以下是对这段 diff 的详细审查和改进建议: 1. 语法逻辑审查ComboBox.qml 修改点:
FlowStyle.qml 修改点:
2. 代码质量改进建议
3. 代码性能建议
4. 代码安全建议
5. 其他建议
总结这段代码实现了扁平化 ComboBox 的功能,整体逻辑正确,但有以下改进空间:
这些改进将使代码更易维护、性能更好且更健壮。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
These changes ensure that flat ComboBox components have proper visual styling with appropriate padding, height, and background colors that respond to user interaction states while maintaining a minimal appearance.
Log: Improved visual styling for flat ComboBox with better padding and interactive states
fix: 调整扁平样式ComboBox的内边距和背景
这些更改确保扁平ComboBox组件具有适当的视觉样式,包括合适的内边距、高度和
响应交互状态的背景颜色,同时保持简约的外观。
Log: 改进了扁平ComboBox的视觉样式,提供更好的内边距和交互状态
PMS: BUG-304333
Summary by Sourcery
Adjust flat ComboBox layout and visual styling for consistent minimal appearance and interactive states.
Enhancements: