Skip to content

fix(shapeswidget): improve text shape handling and logging#835

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-4-16
May 7, 2026
Merged

fix(shapeswidget): improve text shape handling and logging#835
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-4-16

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

  • Clear selection state when switching to text shape to ensure immediate text input on first click.
  • Enhance logging in mouse press events for better debugging of shape interactions and text editing states.

This update addresses user experience issues related to shape selection and text input, ensuring a smoother workflow when using the text tool.

bug: https://pms.uniontech.com/bug-view-359843.html

- Clear selection state when switching to text shape to ensure immediate text input on first click.
- Enhance logging in mouse press events for better debugging of shape interactions and text editing states.

This update addresses user experience issues related to shape selection and text input, ensuring a smoother workflow when using the text tool.

bug: https://pms.uniontech.com/bug-view-359843.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段。这是一个很好的重构,将特定于文本工具切换的逻辑封装到了单独的函数中。让我提供一些改进建议:

代码审查意见

1. 语法逻辑

  • 代码逻辑正确,将文本工具切换时的特殊处理分离到了独立函数中
  • resetForTextToolSwitch() 函数名清晰表达了其用途

2. 代码质量

  • 建议在头文件中添加函数注释,说明该函数的用途和行为
  • 考虑将 Unknow 改为 Unknown(拼写错误)
  • 建议使用枚举类或常量代替硬编码的字符串 "text"

3. 代码性能

  • 当前实现已经很好,没有明显的性能问题
  • 函数调用开销可以忽略不计

4. 代码安全

  • 没有明显的安全问题
  • 建议考虑添加空指针检查(如果 clearSelected() 可能访问空指针)

改进建议

// shapeswidget.h
class ShapesWidget : public QWidget {
    Q_OBJECT
    // ... 其他代码 ...
    
private:
    /**
     * @brief 重置状态以切换到文本工具
     * 
     * 当切换到文本工具时,清除所有选中状态并重置相关标志位,
     * 确保首次点击即可进入文本输入模式。
     */
    void resetForTextToolSwitch();
    
    // 建议使用常量代替硬编码字符串
    static const QString TEXT_TOOL_TYPE;
    
    // ... 其他代码 ...
};

// shapeswidget.cpp
const QString ShapesWidget::TEXT_TOOL_TYPE = "text";

void ShapesWidget::setCurrentShape(QString shapeType)
{
    qDebug() << __FUNCTION__ << __LINE__ << "type: " << shapeType;
    m_currentType = shapeType;
    
    if (shapeType == TEXT_TOOL_TYPE) {
        resetForTextToolSwitch();
    } else {
        setAllTextEditReadOnly();
    }
}

void ShapesWidget::resetForTextToolSwitch()
{
    // 切换到 text 时仍保留上一个图形的选中态,
    // 导致第一次点击先"清选中",第二次点击才进入文本创建。
    // 这里在工具切换时清理选中态,保证首次点击直接进入文本输入。
    clearSelected();
    
    // 重置所有状态标志
    m_isRotated = false;
    m_isResize = false;
    m_clickedKey = Unknown;  // 修正拼写错误
}

其他建议

  1. 考虑使用枚举类来表示工具类型,而不是字符串:
enum class ToolType {
    Text,
    Rectangle,
    Circle,
    // ... 其他工具类型
};

void ShapesWidget::setCurrentShape(ToolType toolType)
{
    m_currentType = toolType;
    if (toolType == ToolType::Text) {
        resetForTextToolSwitch();
    } else {
        setAllTextEditReadOnly();
    }
}
  1. 如果 clearSelected() 可能访问空指针,建议添加检查:
void ShapesWidget::resetForTextToolSwitch()
{
    if (m_selectedItem) {  // 假设 m_selectedItem 是选中的图形
        clearSelected();
    }
    // ... 其他代码 ...
}
  1. 考虑添加单元测试来验证 resetForTextToolSwitch() 的行为是否符合预期。

总体来说,这是一个很好的重构,提高了代码的可读性和可维护性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

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

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot deepin-bot Bot merged commit 90e110a into linuxdeepin:master May 7, 2026
10 checks passed
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