fix(modal): [modal] empty str title should not be visible #1858
Conversation
WalkthroughThe recent updates enhance the modal component's functionality and associated tests within the Vue framework. Key improvements include renaming functions for clarity, introducing new button functionalities, and modifying rendering logic to ensure elements only appear when necessary. Additionally, test cases have been expanded to cover scenarios involving custom and absence of titles, improving the robustness and flexibility of the modal presentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal
participant Title
User->>Modal: Clicks custom button
Modal->>Title: Trigger customClick()
Title-->>Modal: Show modal with custom title
User->>Modal: Clicks no title button
Modal->>Title: Trigger noTitleClick()
Title-->>Modal: Show modal with empty title alert
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 as PR comments)
Additionally, you can add 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
Outside diff range, codebase verification and nitpick comments (1)
examples/sites/demos/pc/app/modal/basic-usage.spec.ts (1)
22-22: Inconsistent Class Name Update:The class name for the success modal SVG element has not been consistently updated across the codebase. The old class name
tiny-icon-successis still present in several files. Please update the following files to use the new class nametiny-modal-svg__successfor consistency:
packages/renderless/src/wizard/vue.tspackages/renderless/types/wizard.type.tsexamples/sites/demos/pc/app/progress/custom-status.vueexamples/sites/demos/pc/app/progress/custom-status-composition-api.vueAnalysis chain
Verify the updated class name for the success modal SVG element.
The change in class name from
tiny-icon-successtotiny-modal-svg__successsuggests a shift in the class naming convention or structure used in the modal component. Ensure that this change is consistent across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the updated class name for the success modal SVG element. # Test: Search for the old and new class names. Expect: Only occurrences of the new class name. rg --type css 'tiny-icon-success' rg --type css 'tiny-modal-svg__success'Length of output: 73
Script:
#!/bin/bash # Description: Verify the updated class name for the success modal SVG element across various file types. # Search for the old class name in CSS, HTML, JS, and TS files rg 'tiny-icon-success' --type css --type html --type js --type ts # Search for the new class name in CSS, HTML, JS, and TS files rg 'tiny-modal-svg__success' --type css --type html --type js --type tsLength of output: 1063
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- examples/sites/demos/pc/app/modal/basic-usage.spec.ts (1 hunks)
- examples/sites/demos/pc/app/modal/lock-view.spec.ts (1 hunks)
- examples/sites/demos/pc/app/modal/title-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/modal/title.spec.ts (1 hunks)
- examples/sites/demos/pc/app/modal/title.vue (2 hunks)
- packages/vue/src/modal/src/mobile.vue (2 hunks)
- packages/vue/src/modal/src/pc.vue (1 hunks)
Additional comments not posted (14)
examples/sites/demos/pc/app/modal/title-composition-api.vue (4)
2-3: Ensure button click handlers are correctly defined.The button click handlers
customClickandnoTitleClickare correctly defined and named. The labels "自定义标题" (Custom Title) and "无标题" (No Title) are appropriate for their respective functionalities.
5-5: Ensure imported components are used correctly.The
ButtonandModalcomponents are correctly imported from@opentiny/vue.
9-11: EnsurecustomClickfunction implements desired functionality.The
customClickfunction correctly displays a modal with the custom title "自定义标题".
13-15: EnsurenoTitleClickfunction implements desired functionality.The
noTitleClickfunction correctly displays a modal with an empty title, implementing the desired functionality.examples/sites/demos/pc/app/modal/title.vue (4)
2-3: Ensure button click handlers are correctly defined.The button click handlers
customClickandnoTitleClickare correctly defined and named. The labels "自定义标题" (Custom Title) and "无标题" (No Title) are appropriate for their respective functionalities.
5-5: Ensure imported components are used correctly.The
ButtonandModalcomponents are correctly imported from@opentiny/vue.
14-16: EnsurecustomClickmethod implements desired functionality.The
customClickmethod correctly displays a modal with the custom title "自定义标题".
18-20: EnsurenoTitleClickmethod implements desired functionality.The
noTitleClickmethod correctly displays a modal with an empty title, implementing the desired functionality.examples/sites/demos/pc/app/modal/title.spec.ts (2)
3-11: Ensure renamed test case implements desired functionality.The test case correctly verifies that the modal displays the custom title "自定义标题".
12-19: Ensure new test case implements desired functionality.The new test case correctly verifies that the modal does not display a title when an empty title is provided.
examples/sites/demos/pc/app/modal/lock-view.spec.ts (1)
8-12: Ensure code example section is expanded before proceeding with test actions.The added steps ensure that the code example section is expanded, which helps to stabilize the test by ensuring the UI is in the correct state before performing further actions. This is a good practice to avoid flaky tests.
packages/vue/src/modal/src/mobile.vue (2)
Line range hint
1-1:
Remove unusedButtoncomponent.The
Buttoncomponent is no longer directly used within this component's scope, which helps to reduce unnecessary dependencies.
108-116: Enhance conditional rendering for modal title.The conditional rendering logic for the modal title ensures that it is only displayed when appropriate, improving the component's responsiveness to its props.
packages/vue/src/modal/src/pc.vue (1)
186-194: LGTM! The conditional rendering of the modal title is correctly implemented.The change ensures that the
spanelement for the modal title is only rendered if thetitleis not an empty string. This prevents empty titles from being displayed, which enhances the user interface.
|
是的，如果不传title就不显示默认标题的话，我认为这是个breaking change，影响比较大
James ***@***.***> 于 2024年8月8日周四 下午2:43写道：
… ***@***.**** commented on this pull request.
------------------------------
In packages/vue/src/modal/src/pc.vue
<#1858 (comment)>:
> @@ -183,13 +183,15 @@ export default defineComponent({
]
)
: null,
- h(
- 'span',
- {
- class: 'tiny-modal__title'
- },
- title || t('ui.alert.title')
- ),
+ title !== ''
如果用户不传title，是否会显示默认标题：‘消息提示’ ?
—
Reply to this email directly, view it on GitHub
<#1858 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKHL5PQJC2Y4P4PNCCSM5SLZQMHQ7AVCNFSM6AAAAABMEMHSFOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMRWHE4DOMRUGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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: #1856
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style