fix: Margin supports configuring negative numbers #1165
Conversation
WalkthroughThis pull request updates the Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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: 0
🧹 Nitpick comments (3)
packages/settings/styles/src/components/spacing/SpacingSetting.vue (2)
8-15: Consider adding visual indication for negative margin values.While the implementation functionally supports negative margins, users might benefit from visual feedback when entering negative values.
<numeric-select v-if="isMargin" :name="property.name" :numericalText="numericalText" :min="-Infinity" + :class="{'negative-value': numericalText < 0}" @update="inputChange" />And add corresponding CSS:
.negative-value :deep(.tiny-numeric__input-inner) { color: var(--te-warning-color, #e6a23c); }
8-15: Consider setting a reasonable minimum instead of -Infinity.Using
-Infinityas the minimum value allows for extremely large negative values that could potentially cause layout issues. Consider setting a reasonable limit like-1000.- :min="-Infinity" + :min="-1000"packages/settings/styles/src/components/inputs/NumericSelect.vue (1)
51-54: Add prop documentation for clarity.Consider adding JSDoc comments to document the purpose of the new min prop, especially its role in supporting negative margins.
+/** + * Minimum allowed value for the numeric input + * Set to a negative value to allow negative numbers (e.g., for margins) + */ min: { type: Number, default: 0 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/settings/styles/src/components/inputs/NumericSelect.vue(2 hunks)packages/settings/styles/src/components/spacing/SpacingSetting.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (5)
packages/settings/styles/src/components/spacing/SpacingSetting.vue (2)
8-14: Appropriate implementation for supporting negative margins.The conditional rendering for margin inputs with
:min="-Infinity"properly enables negative margin values as intended by the PR objective.
15-15: Good separation of padding controls from margin controls.Maintaining a separate NumericSelect instance for padding without modifying its minimum value preserves the expected behavior that padding should remain non-negative.
packages/settings/styles/src/components/inputs/NumericSelect.vue (3)
9-9: Correctly binds the min attribute to the new prop.The binding allows the minimum value to be customized externally, which is essential for supporting negative margins.
51-54: Well-implemented min property with appropriate default.The prop definition maintains backward compatibility with the default value of 0 while allowing consumers to override it.
78-95:❓ Verification inconclusive
Consider updating validation logic for negative values.
The current
changemethod doesn't have special handling for negative values. You might want to add explicit validation or formatting for negative numbers.
🏁 Script executed:
#!/bin/bash # Check if there are any specific validation rules for negative values in related components # Look for any existing validation for numeric values echo "Searching for existing numeric validation logic in related files..." rg -A 5 "validate|isValid|check" --glob "packages/settings/styles/src/**/*.vue" # Check if there are any usages of this component with negative values echo "Checking usage of NumericSelect with negative values..." rg "numeric-select.*min" --glob "packages/settings/styles/src/**/*.vue"Length of output: 5693
Attention: Review the handling of negative values in NumericSelect.vue.
After verifying the existing code and related numeric validation logic, there’s no explicit handling for negative values in the
changemethod. If negative numbers are intended to be valid inputs—with or without the appended "px" suffix—please consider either:• Adding explicit validation and formatting logic for negative values,
• Or clearly documenting the expected behavior if negative numbers should be rejected or handled differently.Please manually verify that the updated logic aligns with behaviors in related components and meets the overall design requirements.
packages/settings/styles/src/components/spacing/SpacingSetting.vue
Outdated
Show resolved
Hide resolved
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
Fixes #1164
margin支持输入负数，常用间距不改变
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
Summary by CodeRabbit