Skip to content

fix(editor): prevent floating toolbar from hiding on button click#349

Merged
pengfeixx merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/floating-toolbar-click-issue
May 28, 2026
Merged

fix(editor): prevent floating toolbar from hiding on button click#349
pengfeixx merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/floating-toolbar-click-issue

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

Use delayed hide (300ms) to avoid QML menu onClosed hiding the toolbar when a toolbar button is clicked. Also prevent default mousedown behavior to keep cursor position unchanged.

使用延迟隐藏(300ms)避免QML菜单关闭时误隐藏悬浮工具栏,
同时阻止mousedown默认行为保持光标位置不变。

Log: 修复悬浮工具栏点击按钮后立即消失的问题
PMS: BUG-361213
Influence: 修复后右键悬浮工具栏上的格式化按钮(加粗、斜体等)
可以正常点击,无选区时点击可切换后续输入的格式状态。

Use delayed hide (300ms) to avoid QML menu onClosed hiding the
toolbar when a toolbar button is clicked. Also prevent default
mousedown behavior to keep cursor position unchanged.

使用延迟隐藏(300ms)避免QML菜单关闭时误隐藏悬浮工具栏,
同时阻止mousedown默认行为保持光标位置不变。

Log: 修复悬浮工具栏点击按钮后立即消失的问题
PMS: BUG-361213
Influence: 修复后右键悬浮工具栏上的格式化按钮(加粗、斜体等)
可以正常点击,无选区时点击可切换后续输入的格式状态。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeex。我已仔细审查了你提供的 git diff,这段代码的主要目的是解决一个常见的UI交互问题:在右键菜单(QML)关闭时触发隐藏悬浮工具栏,会导致用户点击悬浮工具栏上的按钮时,工具栏过早消失的问题。 你通过引入延迟隐藏和 mousedown 事件取消定时器的机制来解决这个问题,思路非常清晰。

不过,从语法逻辑、代码质量、性能和安全性角度来看,仍有以下可以改进的地方:

1. 语法与逻辑

  • 潜在的内存泄漏与事件解绑:你使用了 $(document).on('mousedown', ...) 进行全局事件委托。在单页应用(SPA)或组件可能被销毁的场景下,如果未正确解绑,会导致事件监听器堆积。虽然在这个特定场景下可能不会频繁销毁 document,但良好的编程习惯应该考虑提供解绑机制。
  • 定时器清理逻辑的边界情况:在 hideRightMenu 中,if (hideToolbarTimer) clearTimeout(hideToolbarTimer); 是好习惯,但 setTimeout 的回调中设置 hideToolbarTimer = null 存在一个微小的逻辑缝隙。如果在 300ms 内再次触发 hideRightMenu,旧的定时器被清除并设为 null,然后新建定时器。这本身没问题,但如果未来有其他地方直接读取 hideToolbarTimer 来判断状态,可能会产生误判。

2. 代码质量

  • 全局变量污染var hideToolbarTimer = null; 暴露在全局作用域(或模块顶层)中,不仅污染作用域,还可能被其他代码意外修改。建议使用闭包或将其封装在特定的命名空间/模块中。
  • 魔法数字:延迟时间 300 是一个硬编码的魔法数字。不同设备/浏览器的响应速度可能不同,建议提取为常量,并添加注释说明为什么是 300ms。
  • 选择器性能'.note-air-popover .note-btn' 每次触发 mousedown 时都会在 DOM 树中进行匹配检查。虽然现代浏览器处理起来很快,但如果该工具栏按钮很多或触发极频繁,仍可优化。

3. 代码性能

  • 事件委托的粒度:监听 document 上的 mousedown 事件意味着页面上任何地方的鼠标按下都会进入该事件处理函数并进行 CSS 选择器匹配。更高效的做法是,如果 .note-air-popover 是一个相对固定的容器,可以将事件委托绑定在该容器上,或者至少在事件处理函数入口处尽早判断 e.target 以减少不必要的执行。

4. 代码安全

  • XSS 风险:当前代码没有直接操作 DOM innerHTML 或注入外部数据,因此没有明显的 XSS 漏洞。
  • 防御性编程$('#summernote').summernote('airPopover.hide') 调用时,如果 #summernote 节点已被移除或 summernote 实例已销毁,可能会抛出异常。在延迟 300ms 执行的回调中,这种风险会增大。建议增加容错判断。

改进后的代码及详细解释

// 建议使用 IIFE (立即执行函数) 或模块作用域封装,避免污染全局变量
(function() {
    // 常量提取,消除魔法数字。300ms 是经验值,给用户点击留出反应时间
    const HIDE_TOOLBAR_DELAY = 300; 

    // 悬浮工具栏延迟隐藏定时器
    let hideToolbarTimer = null;

    // 监听悬浮工具栏按钮的mousedown事件
    // 使用事件委托,注意这里的处理逻辑
    $(document).on('mousedown', '.note-air-popover .note-btn', function(e) {
        // 阻止浏览器默认行为,防止焦点从编辑区移开导致选区丢失
        e.preventDefault();
        
        // 清除延迟隐藏定时器
        if (hideToolbarTimer) {
            clearTimeout(hideToolbarTimer);
            hideToolbarTimer = null;
        }
    });

    // 右键隐藏悬浮工具栏(延迟执行)
    window.hideRightMenu = function() { // 如果需要外部调用,可挂载到特定命名空间而非直接覆盖全局
        if (hideToolbarTimer) {
            clearTimeout(hideToolbarTimer);
        }
        
        hideToolbarTimer = setTimeout(function() {
            // 防御性编程:确保 summernote 实例仍然存在
            const $summernote = $('#summernote');
            if ($summernote.length && $summernote.data('summernote')) {
                try {
                    $summernote.summernote('airPopover.hide');
                } catch (err) {
                    console.error('Failed to hide summernote airPopover:', err);
                }
            }
            hideToolbarTimer = null;
        }, HIDE_TOOLBAR_DELAY);
    };
})();

主要改进点说明:

  1. 作用域封装:使用 IIFE 将 hideToolbarTimerHIDE_TOOLBAR_DELAY 封装起来,防止全局变量污染和被外部恶意/误修改。
  2. 常量提取:将 300 提取为 HIDE_TOOLBAR_DELAY 常量,提高代码可读性和可维护性。
  3. 增加防御性编程:在 setTimeout 的回调中,增加了对 #summernote 节点是否存在、是否还挂载着 summernote 实例的判断,并加入了 try-catch。这在异步延迟执行的代码中非常重要,可以防止组件销毁后引发的 JS 报错导致页面卡死。
  4. 变量声明优化:将 var 改为了 let,更符合现代 ES6+ 规范,避免变量提升带来的潜在困扰。

额外建议:如果 hideRightMenu 是被外部(如 QML 的 WebChannel)调用的,建议通过统一的接口对象暴露,而不是直接挂载到 window 上,例如 window.EditorBridge = { hideRightMenu: function() {...} },这样更安全且易于管理。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pengfeixx pengfeixx merged commit fe90281 into linuxdeepin:develop/snipe May 28, 2026
19 of 21 checks passed
@pengfeixx pengfeixx deleted the fix/floating-toolbar-click-issue branch May 28, 2026 06:35
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.

3 participants