-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(notify): [notify] fix notify's demos #2326
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ export default { | |
baseClick() { | ||
Notify({ | ||
type: 'info', | ||
title: (h, params) => <h4>通知消息的标题</h4>, | ||
title: '通知消息的标题', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification CSS Styles Adjustment Required Inline styles are applied to
Please ensure that the notification title's appearance remains consistent by updating or relocating these styles accordingly. 🔗 Analysis chainLGTM! Simplified title definition. The change from a JSX element to a string for the Please verify that this change doesn't negatively impact the appearance of the notification title. If any specific styling was applied to the To verify the appearance, you can run the following script: If the script returns any results, you may need to adjust the styling to maintain the desired appearance with the new string-based title. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any CSS styles applied to h4 elements in notification titles
# Search for CSS rules targeting h4 elements in notify-related styles
rg -i '\.notify.*h4' --type css
# Search for any inline styles on h4 elements within notify components
rg '<h4.*style=' --type vue
Length of output: 91 Script: #!/bin/bash
# Description: Check for any CSS styles applied to h4 elements in notification titles
# Search for CSS rules targeting h4 elements in notify-related styles
rg -i '\.notify.*h4' --type css
# Search for any inline styles on h4 elements within notify components
rg '<h4.*style='
Length of output: 278 |
||
message: '通知消息的正文,通知消息的正文,通知消息的正文,通知消息的正文,通知消息的正文,通知消息的正文', | ||
showIcon: false, | ||
position: 'top-right' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ import { Notify, Button as TinyButton } from '@opentiny/vue' | |
function handleClick() { | ||
Notify({ | ||
type: 'info', | ||
title: (h, params) => <h4>通知消息的标题</h4>, | ||
title: '通知消息的标题', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Inconsistent The 🔗 Analysis chainSimplified title property in Notify function The change from a JSX element to a string for the However, consider the following points:
To ensure consistency across notify demos, run the following script: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for consistent title property usage in notify demos
# Test: Search for title property in notify demos
rg -U --multiline 'Notify\({[^}]*title:.*?[^}]*\)' examples/sites/demos/pc/app/notify
Length of output: 209 Script: #!/bin/bash
# Description: Check for consistent title property usage in notify demos
# Test: Search for title property in Notify function
rg -U --multiline 'Notify\(\{[^}]*title:\s*[^}]*\)' examples/sites/demos/pc/app/notify
Length of output: 4832 Script: #!/bin/bash
# Description: Check for consistent title property usage in notify demos
# Test: Search for title property in Notify function
rg -U --multiline 'Notify\(\{[^}]*title:\s*[^}]*\}\)' examples/sites/demos/pc/app/notify
Length of output: 17199 |
||
message: '通知消息的正文,通知消息的正文,通知消息的正文,通知消息的正文,通知消息的正文,通知消息的正文', | ||
position: 'top-right', | ||
customClass: 'my-custom-cls', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Notification duration too short
The
duration
property is set to500ms
, which may be insufficient for users to read the notification message. Consider increasing it to a higher value (e.g.,1000ms
) to enhance user experience.examples/sites/demos/pc/app/notify/duration-composition-api.vue: duration: 500
🔗 Analysis chain
Simplified title improves readability.
The change from a JSX element to a simple string for the
title
property enhances code clarity and maintainability.However, the 500ms duration might be too short for users to read the notification. Consider increasing it to ensure better user experience. Run the following script to check for other short durations:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 171
Script:
Length of output: 1440