Skip to content

fix: 🐛 preserve global dialog alert buttons#70

Open
Guan-Meng-Yuan wants to merge 1 commit into
wot-ui:v2from
Guan-Meng-Yuan:codex/global-dialog-options
Open

fix: 🐛 preserve global dialog alert buttons#70
Guan-Meng-Yuan wants to merge 1 commit into
wot-ui:v2from
Guan-Meng-Yuan:codex/global-dialog-options

Conversation

@Guan-Meng-Yuan

Copy link
Copy Markdown

🤔 这个 PR 的性质是?(至少选择一个)

  • 日常 bug 修复
  • 新特性提交
  • 站点、文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • TypeScript 定义更新
  • CI/CD 改进
  • 包体积优化
  • 性能优化
  • 功能增强
  • 国际化改进
  • 代码重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他改动(是关于什么的改动?)

🔗 相关 Issue

暂无。

💡 需求背景和解决方案

GlobalDialog 的全局封装会统一调用 dialog.show(option),并在写入状态时补充默认按钮配置。原逻辑会导致 alert() 设置的 showCancelButton: false 被默认的 cancelButtonProps 影响,取消按钮仍然可能显示;同时 confirmButtonText / cancelButtonText 等快捷配置也没有统一合并到实际按钮配置中。

本次修复:

  1. useGlobalDialog 内统一规范化 show / alert / confirm / prompt 的 dialog options。
  2. alert 默认隐藏取消按钮,confirm / prompt 默认显示取消按钮,同时保留调用方显式传入的 showCancelButton
  3. confirmButtonText / cancelButtonText 合并进对应 button props,并在 showCancelButton === false 时保留 cancelButtonProps: null
  4. 保持 GlobalDialog.vue 作为展示层,不在组件内分发不同 dialog 类型。

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6ecf5a9-b3cc-4cf9-a606-cfd59190d495

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the useGlobalDialog composable by introducing helper functions to centralize dialog option normalization and button property processing. The review feedback suggests improving the flexibility of button styling by allowing the round property to be overridden and recommends further refactoring of the store actions to eliminate duplicated logic across the show, alert, confirm, and prompt methods.

Comment on lines +26 to +30
return {
...props,
...(text ? { text } : {}),
round: false,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation forces round: false by placing it after the spread operator, which prevents users from overriding this style (e.g., by passing confirmButtonProps: { round: true }). Moving the default property before the spread allows for user customization while still providing the desired default.

Suggested change
return {
...props,
...(text ? { text } : {}),
round: false,
}
return {
round: false,
...props,
...(text ? { text } : {}),
}

Comment on lines 92 to 107
show(option: GlobalDialogOptions | string) {
this.currentPage = getCurrentPath()
this.dialogOptions = {
...(CommonUtil.isString(option) ? { title: option } : option),
cancelButtonProps: {
round: false,
},
confirmButtonProps: {
round: false,
},
}
this.dialogOptions = normalizeOption(option)
},
alert(option: GlobalDialogOptions | string) {
const DialogOptions = CommonUtil.deepMerge({ type: 'alert' }, CommonUtil.isString(option) ? { title: option } : option) as DialogOptions
DialogOptions.showCancelButton = false
this.show(DialogOptions)
this.currentPage = getCurrentPath()
this.dialogOptions = normalizeOption(option, 'alert')
},
confirm(option: GlobalDialogOptions | string) {
const DialogOptions = CommonUtil.deepMerge({ type: 'confirm' }, CommonUtil.isString(option) ? { title: option } : option) as DialogOptions
DialogOptions.showCancelButton = true
this.show(DialogOptions)
this.currentPage = getCurrentPath()
this.dialogOptions = normalizeOption(option, 'confirm')
},
prompt(option: GlobalDialogOptions | string) {
const DialogOptions = CommonUtil.deepMerge({ type: 'prompt' }, CommonUtil.isString(option) ? { title: option } : option) as DialogOptions
DialogOptions.showCancelButton = true
this.show(DialogOptions)
this.currentPage = getCurrentPath()
this.dialogOptions = normalizeOption(option, 'prompt')
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for setting currentPage and calling normalizeOption is duplicated across all dialog actions. Refactoring the show action to accept an optional type parameter and having alert, confirm, and prompt call this.show would improve maintainability and reduce code duplication.

    show(option: GlobalDialogOptions | string, type?: DialogType) {
      this.currentPage = getCurrentPath()
      this.dialogOptions = normalizeOption(option, type)
    },
    alert(option: GlobalDialogOptions | string) {
      this.show(option, 'alert')
    },
    confirm(option: GlobalDialogOptions | string) {
      this.show(option, 'confirm')
    },
    prompt(option: GlobalDialogOptions | string) {
      this.show(option, 'prompt')
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant