Skip to content

fix: emit valueChanged signals for List and Optional widgets#103

Draft
hthienloc wants to merge 1 commit into
fcitx:masterfrom
hthienloc:fix-list-option-widget
Draft

fix: emit valueChanged signals for List and Optional widgets#103
hthienloc wants to merge 1 commit into
fcitx:masterfrom
hthienloc:fix-list-option-widget

Conversation

@hthienloc

Copy link
Copy Markdown

Description

This PR addresses issue #100 by ensuring that ListOptionWidget and OptionalOptionWidget correctly notify the parent configuration dialog when their values change.

Previously, these widgets modified their internal state/model but failed to emit the OptionWidget::valueChanged() signal. This caused the "Apply" and "OK" buttons in the main configuration window to remain disabled, effectively preventing users from saving changes to these fields unless another unrelated setting was also modified.

Changes

  • ListOptionWidget: Connected rowsInserted, rowsRemoved, dataChanged, and rowsMoved signals from the internal model to valueChanged().
  • OptionalOptionWidget: Connected the checkbox state change and sub-widget's valueChanged signal to the parent's valueChanged().

Evidence

In src/lib/configwidgetslib/optionwidget.cpp, all other basic widgets (Integer, String, Boolean, Enum) already follow this pattern to enable change detection:

// Example from StringOptionWidget
connect(lineEdit_, &QLineEdit::textChanged, this, &OptionWidget::valueChanged);

This PR brings ListOptionWidget and OptionalOptionWidget into consistency with the rest of the library.

Fixes #100

- Connect model mutation signals (rowsInserted, rowsRemoved, dataChanged, rowsMoved) to OptionWidget::valueChanged() in ListOptionWidget.
- Connect checkbox state and sub-widget changes to OptionWidget::valueChanged() in OptionalOptionWidget.

Fixes fcitx#100
@hthienloc hthienloc marked this pull request as draft April 8, 2026 11:25
@hthienloc

Copy link
Copy Markdown
Author

I need to test this carefully.

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.

ListOptionWidget does not emit valueChanged() — Apply button stays disabled after list modifications

1 participant