fix(modal): add attributes (cancel-control, confirm-control) #41
Conversation
James-9696
changed the title
WalkthroughThis pull request adds customizable confirmation and cancel button text to the TinyModal component. It includes two new demo components showcasing the feature, updates the modal component's rendering logic to support custom button labels with fallback to translations, refines modal footer styling to use flexbox layout, and introduces corresponding documentation and CSS variables. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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)
packages/mobile/components/modal/src/mobile.vue (1)
208-209: Refactor class binding to use object syntax.The conditional class binding works but can be more idiomatic. Adding an empty string to a class array is less clean than using Vue's object-based class binding.
Apply this diff:
{ props: { type: 'primary', size: 'small', - class: [type !== 'confirm' ? 'tiny-mobile-button__single' : ''] }, + class: { 'tiny-mobile-button__single': type !== 'confirm' }, on: { click: this.confirmEvent }packages/demos/app/modal/custom-btn.vue (1)
8-9: Simplify static prop bindings.The props are using v-bind (
:) with string literals, which is unnecessary for static strings. This is less idiomatic and slightly less performant than passing them as regular attributes.Apply this diff:
<tiny-modal v-model="value1" type="confirm" show-footer - :confirm-content="'OK'" - :cancel-content="'你好'" + confirm-content="OK" + cancel-content="你好" >Note: Only use v-bind (
:) when binding to variables, computed properties, or expressions—not for static string literals.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/demos/app/modal/custom-btn.vue(1 hunks)packages/demos/app/modal/status.vue(1 hunks)packages/demos/app/modal/webdoc/modal.js(3 hunks)packages/mobile/components/modal/src/mobile.vue(3 hunks)packages/theme-mobile/src/modal/index.less(1 hunks)packages/theme-mobile/src/modal/vars.less(1 hunks)
🔇 Additional comments (9)
packages/demos/app/modal/status.vue (1)
9-27: LGTM! Clean demo implementation.The new status demo section is well-structured, properly imports the required dependencies, and correctly demonstrates the modal with a status icon.
packages/mobile/components/modal/src/mobile.vue (3)
43-48: LGTM! Well-structured button text computation.The button text fallback chain is correctly implemented using nullish coalescing, providing a clean priority: custom content prop → button props text → localized default.
104-120: LGTM! Flexible status rendering.The conditional status rendering correctly handles both string-based status icons and custom status components, providing good flexibility for consumers.
193-200: LGTM! Cancel button properly configured.The button configuration correctly uses the computed
cancelButtonTextand applies consistent sizing with the updated design.packages/theme-mobile/src/modal/vars.less (1)
27-27: LGTM! CSS variable addition follows conventions.The new button spacing variable correctly references the existing design token and follows the established naming pattern.
packages/theme-mobile/src/modal/index.less (2)
393-394: LGTM! Improved footer layout approach.Switching from text-align to flexbox provides better control over button positioning and is more semantically correct for this use case.
397-397: Review button spacing - margin applies to all buttons including the first.The current implementation applies
margin-leftto all buttons via the.@{button-prefix-cls}selector. Withjustify-content: center, this could cause asymmetric spacing since the first button will also have left margin.Consider using an adjacent sibling selector to only apply margin between buttons:
.@{button-prefix-cls} { - margin-left: var(--tvm-Modal-btn-space); + & + & { + margin-left: var(--tvm-Modal-btn-space); + } &__single {Please verify the rendered layout to ensure the button spacing is symmetric and centered as intended.
packages/demos/app/modal/webdoc/modal.js (2)
29-48: LGTM! Comprehensive prop documentation.The new
confirm-contentandcancel-contentprops are well-documented with clear descriptions, appropriate types, and default values in both Chinese and English.
623-638: LGTM! Clear demo documentation.The custom-btn demo entry provides clear descriptions and proper code file references for the new button customization feature.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
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
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.