fix:button group menu item height#3129
Conversation
WalkthroughThis pull request updates multiple components in the project. In the button group configurator, a fragment wrapper is replaced with an Ant Design Flex component, and the configuration object is enhanced with additional background and shadow properties. Meanwhile, in several designer components, the Changes
Sequence Diagram(s)sequenceDiagram
participant BG as ButtonGroupInner
participant CP as ConfigProvider
participant M as Menu
BG->>CP: Wrap Menu with theme configuration
CP->>M: Render Menu with itemHeight CSS variable
sequenceDiagram
participant SI as SettingsInput.Factory
participant M as Model
SI->>M: Evaluate model.hidden
alt Hidden is true
SI->>SI: Return null (skip rendering)
else Hidden is false
SI->>SI: Render SettingInput with props
end
Possibly related PRs
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)
🪧 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 (2)
shesha-reactjs/src/providers/theme/index.tsx (2)
57-61: Consider using proper TypeScript types instead ofany.While the menu item height customization looks good, using
as anyis generally considered a code smell as it bypasses TypeScript's type checking. This could lead to potential issues if the API changes.Consider using a more specific type or checking the Ant Design's theme typing documentation for the correct type for
itemHeight.- itemHeight: 'clamp(40px, 40px, 100%)' as any + itemHeight: 'clamp(40px, 40px, 100%)'If TypeScript still complains about the type, check if there's a more specific type you can use or if there's a reason the string value needs a type conversion.
59-59: Review the CSS clamp function usage.The current CSS
clamp(40px, 40px, 100%)function has the minimum and preferred values set to the same value (40px). This makes the clamp function behave essentially asmax(40px, 100%), which could be simplified.If the intention is to have a fixed 40px height, consider using just
40px. If you want to allow flexibility, make sure the preferred value is different from the minimum.For example:
- itemHeight: 'clamp(40px, 40px, 100%)' as any + itemHeight: '40px' as anyOr if you want flexibility between 40px and container height:
- itemHeight: 'clamp(40px, 40px, 100%)' as any + itemHeight: 'clamp(40px, 50px, 100%)' as any
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
shesha-reactjs/src/designer-components/button/buttonGroup/buttonGroup.tsx(2 hunks)shesha-reactjs/src/designer-components/button/buttonGroup/styles/styles.ts(3 hunks)shesha-reactjs/src/providers/theme/index.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- shesha-reactjs/src/designer-components/button/buttonGroup/buttonGroup.tsx
- shesha-reactjs/src/designer-components/button/buttonGroup/styles/styles.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-attempt
🔇 Additional comments (1)
shesha-reactjs/src/providers/theme/index.tsx (1)
57-61: Good implementation of menu item height customization.This change appropriately leverages Ant Design's theme customization system to set a consistent height for menu items. This aligns well with the PR objective to fix button group menu item height issues.
The approach of using theme customization rather than direct CSS overrides is cleaner and more maintainable, as it works with Ant Design's component API rather than fighting against it.
2 participants
Summary by CodeRabbit
Style
New Features