feat(style-panel): modify this style-panel styles according to the design craft #853
Conversation
…iny-engine into refactor/style-panel
…iny-engine into refactor/style-panel
…iny-engine into refactor/style-panel
WalkthroughThis pull request introduces several modifications across various Vue components within the configurator and settings packages. Key changes include enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
packages/settings/styles/src/components/inputs/NumericSelect.vue (2)
53-54: Improve null value handling.The initialization with
nullis good, but consider adding type validation to ensure consistent behavior.-const numericalModelValue = ref(props.numericalText || null) +const numericalModelValue = ref(props.numericalText !== undefined ? String(props.numericalText) : null)
113-123: Consider improving the numeric input styling.The current styles could be enhanced for better visual hierarchy and consistency.
:deep(.tiny-numeric) { width: 100%; .tiny-numeric__input-inner { padding-right: 22px; + height: 24px; + line-height: 24px; } .tiny-numeric__unit { font-size: 12px; color: var(--te-common-text-weaken); background-color: var(--te-common-bg-default); + padding: 0 4px; + border-radius: 0 2px 2px 0; } }packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue (4)
18-31: Extract popover component to reduce code duplicationThe popover logic is duplicated across multiple places in the template. Consider extracting it into a reusable component.
Create a new component
TooltipLabel.vue:<template> <span v-if="label && !content">{{ label }}</span> <tiny-popover v-if="content" :effect="effect" :placement="placement" :visible-arrow="false" :content="content" trigger="hover" > <template #reference> <span v-if="label">{{ label }}</span> <svg-icon v-if="icon" :name="icon" class="bem-Svg"></svg-icon> </template> </tiny-popover> </template> <script setup> defineProps({ label: String, content: String, icon: String, effect: String, placement: String }) </script>Also applies to: 72-85
96-103: Optimize imports for better tree-shakingConsider importing components directly from their respective modules.
Apply this diff:
-import { - Popover as TinyPopover, - ButtonGroup as TinyButtonGroup, - Button as TinyButton, - Dropdown as TinyDropdown, - DropdownMenu as TinyDropdownMenu, - DropdownItem as TinyDropdownItem -} from '@opentiny/vue' +import TinyPopover from '@opentiny/vue/popover' +import TinyButtonGroup from '@opentiny/vue/button-group' +import TinyButton from '@opentiny/vue/button' +import TinyDropdown from '@opentiny/vue/dropdown' +import TinyDropdownMenu from '@opentiny/vue/dropdown-menu' +import TinyDropdownItem from '@opentiny/vue/dropdown-item'
141-143: Improve type handling in getItemWidthThe function unnecessarily parses a number prop back to integer.
Apply this diff:
const getItemWidth = (collapsed = false) => { - return `${parseInt(props.labelWidth, 10) + (collapsed ? 15 : 0)}` + return props.labelWidth + (collapsed ? 15 : 0) }
190-196: Improve style maintainability with CSS variablesUse CSS variables for better theme management and maintainability.
Apply this diff:
+:root { + --tab-height: 24px; + --tab-font-size: 12px; + --tab-border-radius: 4px; + --tab-max-width: 210px; + --tab-z-index: 9999; +} .tab-container { - max-width: 210px; - height: 24px; - font-size: 12px; + max-width: var(--tab-max-width); + height: var(--tab-height); + font-size: var(--tab-font-size); background-color: var(--ti-lowcode-setting-style-tab-bg-color); display: flex; - border-radius: 4px; + border-radius: var(--tab-border-radius); } .tiny-dropdown-menu { // ... - z-index: 9999; + z-index: var(--tab-z-index); box-shadow: 0 0 10px 0 var(--te-common-border-default); }Also applies to: 273-274
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
packages/common/component/MetaCodeEditor.vue(3 hunks)packages/configurator/src/color-configurator/ColorConfigurator.vue(4 hunks)packages/configurator/src/slider-configurator/SliderConfigurator.vue(2 hunks)packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue(1 hunks)packages/settings/styles/src/components/inputs/NumericSelect.vue(4 hunks)packages/theme/dark/variable.less(1 hunks)packages/theme/light/variable.less(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/theme/dark/variable.less
- packages/theme/light/variable.less
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/common/component/MetaCodeEditor.vue
- packages/configurator/src/color-configurator/ColorConfigurator.vue
- packages/configurator/src/slider-configurator/SliderConfigurator.vue
🔇 Additional comments (4)
packages/settings/styles/src/components/inputs/NumericSelect.vue (3)
3-16:
Improve accessibility and user experience of the numeric input.
The current implementation has several UX concerns:
- Hiding the unit on hover reduces accessibility and usability
- Using hover state on touch devices is problematic
Remove the hover-based unit visibility:
<tiny-numeric
v-model="numericalModelValue"
:placeholder="placeholder"
controls-position="right"
:unit="numericalSuffix"
:empty-value="null"
allow-empty
- @mouseover="isNumericHover = true"
- @mouseleave="isNumericHover = false"
@focus="focus = true"
@blur="focus = false"
@change="change"
>
</tiny-numeric>59-62:
Remove hover-dependent suffix logic.
The suffix should always be visible for better accessibility and user experience.
const numericalSuffix = computed(() => {
const val = String(numericalModelValue.value).toLowerCase()
- if (isNumericHover.value) {
- return ''
- }
return val === 'auto' || val === 'none' || percentageReg(val) ? '-' : props.suffix
})125-134:
Remove redundant suffix styles.
These styles are now handled by the tiny-numeric component and should be removed to prevent conflicts.
-.suffix-wrap {
- .suffix-text {
- font-size: 12px;
- color: var(--te-common-text-weaken);
- }
-
- &:hover {
- .suffix-text {
- display: none;
- }
- }
-}packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue (1)
105-130: 🛠️ Refactor suggestion
Add validation for props
The props lack proper validation which could lead to runtime errors.
Apply this diff:
const props = defineProps({
modelValue: {
type: String,
default: ''
},
valueKey: {
type: String,
default: ''
},
effect: {
type: String,
+ validator: (value) => ['dark', 'light'].includes(value),
default: 'dark'
},
placement: {
type: String,
+ validator: (value) => [
+ 'top', 'top-start', 'top-end',
+ 'bottom', 'bottom-start', 'bottom-end'
+ ].includes(value),
default: 'top'
},
labelWidth: {
type: Number,
+ validator: (value) => value > 0,
default: 63
},
options: {
type: Array,
default: () => [],
+ validator: (value) => value.every(item =>
+ typeof item === 'object' &&
+ 'value' in item &&
+ (item.label !== undefined || item.icon !== undefined)
+ )
}
})Likely invalid or redundant comment.
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
feat(style-panel): add expand icon of style collapsed tab
feat(style-panel): add collapsed icon of style panel collapsed tabs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/settings/styles/src/components/inputs/NumericSelect.vue (1)
67-71: Improve watch handler for numericalText.The current watch handler might cause unnecessary updates and doesn't handle edge cases properly.
watch( () => props.numericalText, (newValue) => { - numericalModelValue.value = newValue || null + // Only update if values are different to prevent unnecessary renders + const currentValue = numericalModelValue.value + const newProcessedValue = newValue || null + if (String(currentValue) !== String(newProcessedValue)) { + numericalModelValue.value = newProcessedValue + } } )packages/settings/styles/src/components/background/BackgroundImageSetting.vue (1)
115-134: Add property validation to prevent runtime errorsWhile the method correctly handles the new data structure, it would benefit from validation to ensure all required properties are present before accessing them.
Consider adding validation:
const selectType = (value) => { + if (!value?.styleComponent) { + console.warn('Missing required styleComponent property') + return + } let styleObj = {} if (value.styleComponent === 'ImageSetting') { + if (!value.imageUrl || !value.position || !value.size) { + console.warn('Missing required ImageSetting properties') + return + } styleObj = { [BACKGROUND_PROPERTY.BackgroundImage]: `url(${value.imageUrl})`, [BACKGROUND_PROPERTY.BackgroundPosition]: value.position, [BACKGROUND_PROPERTY.BackgroundSize]: value.size } } else { + if (!value.imageUrl) { + console.warn('Missing required imageUrl property') + return + } styleObj = { [BACKGROUND_PROPERTY.BackgroundImage]: value.imageUrl, [BACKGROUND_PROPERTY.BackgroundPosition]: null, [BACKGROUND_PROPERTY.BackgroundSize]: null, [BACKGROUND_PROPERTY.BackgroundRepeat]: null, [BACKGROUND_PROPERTY.BackgroundAttachment]: null } }packages/settings/styles/src/components/background/ImageSetting.vue (1)
147-156: Consider optimizing setBackgroundSize methodThe method could be simplified by extracting the string transformation logic into a helper function to reduce code duplication.
+ const formatDimension = (value, suffix) => { + const lowered = value.toLowerCase() + return lowered === 'auto' ? lowered : `${lowered}${suffix}` + } const setBackgroundSize = () => { - const isAutoWidth = state.width.toLocaleLowerCase() === 'auto' - const isAutoHeight = state.height.toLocaleLowerCase() === 'auto' - const width = isAutoWidth ? state.width.toLocaleLowerCase() : `${state.width.toLocaleLowerCase() + state.widthSuffix}` - const height = isAutoHeight - ? state.height.toLocaleLowerCase() - : `${state.height.toLocaleLowerCase() + state.heightSuffix}` + const width = formatDimension(state.width, state.widthSuffix) + const height = formatDimension(state.height, state.heightSuffix) updateStyle({ [BACKGROUND_PROPERTY.BackgroundSize]: `${width} ${height}` }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/design-core/assets/style-panel-collapsed.svgis excluded by!**/*.svgpackages/design-core/assets/style-panel-expand.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
packages/configurator/src/color-configurator/ColorConfigurator.vue(4 hunks)packages/configurator/src/number-configurator/NumberConfigurator.vue(3 hunks)packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue(1 hunks)packages/layout/src/DesignSettings.vue(4 hunks)packages/settings/styles/src/components/background/BackgroundGroup.vue(11 hunks)packages/settings/styles/src/components/background/BackgroundImageSetting.vue(6 hunks)packages/settings/styles/src/components/background/ImageSetting.vue(8 hunks)packages/settings/styles/src/components/inputs/NumericSelect.vue(4 hunks)packages/theme/light/setting-style.less(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/configurator/src/color-configurator/ColorConfigurator.vue
- packages/configurator/src/number-configurator/NumberConfigurator.vue
- packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
- packages/layout/src/DesignSettings.vue
- packages/settings/styles/src/components/background/BackgroundGroup.vue
- packages/theme/light/setting-style.less
🔇 Additional comments (9)
packages/settings/styles/src/components/inputs/NumericSelect.vue (1)
111-124: Improve transitions and color contrast.
The current styles have a few issues:
- The transition property only specifies duration without properties
- The background color variable might not provide sufficient contrast
transition: 0.3s;
+ transition: border-color 0.3s, background-color 0.3s;
&.focus {
border-color: var(--te-common-border-default);
}
:deep(.tiny-numeric) {
width: 100%;
.tiny-numeric__input-inner {
padding-right: 22px;
+ // Ensure sufficient contrast with background
+ background-color: var(--te-common-bg-primary);
}packages/settings/styles/src/components/background/BackgroundImageSetting.vue (4)
5-13: LGTM: Clean implementation of TabsGroupConfigurator
The replacement of tooltip-based selection with TabsGroupConfigurator is well implemented with proper prop bindings and event handling.
30-30: LGTM: Proper import and component registration
The TabsGroupConfigurator is correctly imported and registered as a component.
Also applies to: 46-46
75-106: LGTM: Well-structured type definitions
The typeLists data structure has been improved with:
- Clear property naming (content, icon)
- Consistent value object structure for each type
- Appropriate default values for required properties
Line range hint 216-225: LGTM: Clean and consistent styling
The line class styles are well-implemented using:
- Project's CSS naming conventions
- Theme variables for colors
- Proper scoping and positioning
packages/settings/styles/src/components/background/ImageSetting.vue (4)
22-27: LGTM! Clean implementation of TabsGroupConfigurator
The TabsGroupConfigurator components are consistently implemented across size, repeat, and fixed options, providing a unified interface for user selections.
Also applies to: 56-61, 65-70
29-50: LGTM! Well-structured width/height controls
The implementation of width and height controls using InputSelect provides good user control with appropriate disable states based on the size selection.
244-245: LGTM! Consistent border styling
The border and border-radius styles are appropriately defined and use the correct CSS variables for consistency.
Also applies to: 329-330
76-77: Verify integration with tiny-engine-configurator
Please ensure that the TabsGroupConfigurator component from @opentiny/tiny-engine-configurator is properly tested with all the options and edge cases.
✅ Verification successful
TabsGroupConfigurator integration appears consistent and well-established
The component is being used consistently across multiple style-related components in the codebase:
- Layout settings (LayoutGroup.vue)
- Typography settings (TypographyGroup.vue)
- Size settings (SizeGroup.vue)
- Effects settings (EffectGroup.vue)
- Border settings (BorderGroup.vue)
- Background settings (ImageSetting.vue, BackgroundImageSetting.vue, RadialGradient.vue)
The package is properly declared as a dependency in all relevant package.json files, including settings/styles. The implementation pattern is consistent across all these components, suggesting established usage patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other usages of TabsGroupConfigurator to ensure consistent implementation
# Search for other TabsGroupConfigurator usages
echo "Checking TabsGroupConfigurator usage patterns:"
rg -A 2 "TabsGroupConfigurator" --type vue
# Search for related test files
echo -e "\nChecking for related test files:"
fd -e spec.js -e test.js -e spec.ts -e test.ts | rg -i "tabsgroupconfigurator|imagesetting"
Length of output: 348
Script:
#!/bin/bash
# Let's try a different approach to search for the component usage and tests
# Search for TabsGroupConfigurator usage with ripgrep without file type restriction
echo "Checking TabsGroupConfigurator usage patterns:"
rg "TabsGroupConfigurator" -l
# Search for imports from tiny-engine-configurator
echo -e "\nChecking imports from tiny-engine-configurator:"
rg "from ['|\"]@opentiny/tiny-engine-configurator['|\"]" -A 2
# Search for test files in a different way
echo -e "\nChecking for related test files:"
find . -type f \( -name "*test*.js" -o -name "*test*.ts" -o -name "*spec*.js" -o -name "*spec*.ts" \) -exec grep -l -i "tabsgroupconfigurator\|imagesetting" {} \;
# Check if the package is properly declared in package.json
echo -e "\nChecking package.json for dependency:"
find . -name "package.json" -exec grep -l "@opentiny/tiny-engine-configurator" {} \;
Length of output: 9178
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Outdated
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
packages/settings/styles/src/Main.vue (1)
132-136: Consider a more declarative approach for handoverGroup.The current implementation mutates the array directly with
shift(). Consider a more declarative approach that creates a new array instead.- const handoverGroup = (actives) => { - if (props.isCollapsed) { - activeNames.value = actives.length > 1 ? actives.shift() : actives - } - } + const handoverGroup = (actives) => { + if (props.isCollapsed) { + activeNames.value = actives.length > 1 ? [actives[0]] : actives + } + }packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue (2)
132-137: Convert refs to computed properties for better reactivity.Some refs could be computed properties to automatically react to changes.
Apply this diff:
-const picked = ref(null) +const picked = ref(props.modelValue) const uncollapsedOptions = computed(() => props.options.filter((option) => !option.collapsed)) const collapsedOptions = computed(() => props.options.filter((option) => option.collapsed)) -const foldsOptions = ref([]) +const foldsOptions = computed(() => + collapsedOptions.value.length > 1 ? collapsedOptions.value.slice(1) : [] +) -const selectedCollapsedOption = ref(null) +const selectedCollapsedOption = computed(() => + collapsedOptions.value.length ? collapsedOptions.value[0] : null +) const isCollapsedSelected = ref(false)
236-267: Add transitions for smoother UX.Add transitions for hover and selected states.
Apply this diff:
.tiny-dropdown { display: flex; justify-content: center; align-items: center; height: 24px; + transition: background-color 0.2s ease, color 0.2s ease; background-color: var(--ti-lowcode-base-bg-5); color: var(--te-common-text-weaken); &.selected { background-color: var(--ti-lowcode-base-gray-101); color: var(--te-common-text-primary); border-radius: 4px; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/design-core/assets/style-panel-collapsed.svgis excluded by!**/*.svgpackages/design-core/assets/style-panel-expand.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
packages/configurator/src/number-configurator/NumberConfigurator.vue(3 hunks)packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue(1 hunks)packages/layout/src/DesignSettings.vue(4 hunks)packages/settings/styles/src/Main.vue(6 hunks)packages/theme/light/setting-style.less(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/configurator/src/number-configurator/NumberConfigurator.vue
- packages/layout/src/DesignSettings.vue
- packages/theme/light/setting-style.less
🔇 Additional comments (3)
packages/settings/styles/src/Main.vue (3)
30-30: LGTM: Event handler addition enhances collapse functionality.
The addition of the @change="handoverGroup" event handler to <tiny-collapse> properly implements the collapsible behavior control.
Line range hint 108-125: LGTM: Well-structured prop and computed property implementation.
The isCollapsed prop and activeNames computed property are well-implemented, providing a clean way to control the collapse state.
264-270: LGTM: Consistent use of CSS variables.
The new .dots class follows the project's CSS variable pattern, using --te-common-border-checked for theming consistency. This aligns with the existing usage of CSS variables throughout the component.
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/settings/styles/src/components/background/ImageSetting.vue (3)
22-27: Enhance accessibility for TabsGroupConfigurator componentsThe new tab group components need ARIA attributes to improve accessibility for screen readers.
Add aria-label attributes to describe the purpose of each tab group:
<tabs-group-configurator :options="BACKGROUND_SIZE_OPTIONS" :modelValue="state.sizeSelected" label-width="67" + aria-label="Background size options" @update:modelValue="selectSize" ></tabs-group-configurator> <tabs-group-configurator :options="REPEAT_OPTIONS" :modelValue="state.repeat" label-width="50" + aria-label="Background repeat options" @update:modelValue="selectRepeat" ></tabs-group-configurator> <tabs-group-configurator :options="FIXED_OPTIONS" :modelValue="state.fixedSelected" label-width="98" + aria-label="Background attachment options" @update:modelValue="selectFixed" ></tabs-group-configurator>Also applies to: 56-61, 65-70
190-197: Add validation to selectRepeat and selectFixed functionsThe functions should validate input values against allowed options to prevent invalid states.
Add validation checks:
const selectRepeat = (value) => { + if (!REPEAT_OPTIONS.some(option => option.value === value)) { + console.warn(`Invalid repeat value: ${value}`) + return + } state.repeat = value updateStyle({ [BACKGROUND_PROPERTY.BackgroundRepeat]: value }) } const selectFixed = (value) => { + if (!FIXED_OPTIONS.some(option => option.value === value)) { + console.warn(`Invalid fixed value: ${value}`) + return + } state.fixedSelected = value updateStyle({ [BACKGROUND_PROPERTY.BackgroundAttachment]: value }) }
244-245: Optimize image-wrap border stylesThe border properties can be consolidated for better performance and maintainability.
Consolidate the border properties:
- border: 1px dotted var(--te-common-border-default); - border-radius: 4px; + border: 1px dotted var(--te-common-border-default); + border-radius: var(--te-border-radius-small, 4px);Consider using a design system variable for border-radius to maintain consistency across components.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
TabsGroupConfiguratorcomponent for improved tabbed interfaces across various settings.NumberConfiguratorwith unit selection and improved layout.tabs-group-configurator.teleportinModalMask.vuefor customizable modal positioning.Improvements
ImageSettingandRadialGradient, by replacing tooltips with tabbed selections.BackgroundGroupandTypographyGroupcomponents.LayoutGroupfor clarity and usability.SpacingGroup.Bug Fixes
NumericSelectto ensure only valid numeric entries are accepted.Chores