FEAT(client): Add toggleable transparency state for talking ui #6907
Conversation
Hartmnt
left a comment
Hartmnt
left a comment
There was a problem hiding this comment.
You have tested this only on Windows so far, correct?
| LOAD(iTalkingUI_PrefixCharCount, "ui/talkingUI_PrefixCharCount"); | ||
| LOAD(iTalkingUI_PostfixCharCount, "ui/talkingUI_PostfixCharCount"); | ||
| LOAD(qsTalkingUI_AbbreviationReplacement, "ui/talkingUI_AbbreviationReplacement"); | ||
| LOAD(iTalkingUI_TransparencyLevel, "ui/talkingUI_TransparencyLevel"); |
There was a problem hiding this comment.
Can be removed. This method loads legacy settings. Since you are creating a new setting, there will be no one with that setting trying to migrate.
| </property> | ||
|
|
||
| <property name="toolTip"> | ||
| <string>Transparency level of the TalkingUI. </string> |
There was a problem hiding this comment.
| <string>Transparency level of the TalkingUI. </string> | |
| <string>Transparency level of the TalkingUI.</string> |
| <property name="accessibleName"> | ||
| <string>Transparency level</string> | ||
| </property> |
There was a problem hiding this comment.
This is a bad accessibleName. This field is used for users with assistive technology such as screen readers. When they tab into this new field they will simply hear "transparency level" and there is no context that this affects the talking UI.
I'd change this and the toolTip to "TalkingUI background transparency level" or something similar
| int iTalkingUI_PostfixCharCount = 2; | ||
| QString qsTalkingUI_AbbreviationReplacement = QStringLiteral("..."); | ||
| std::optional< QColor > talkingUI_BackgroundColor = std::nullopt; | ||
| int iTalkingUI_TransparencyLevel = 255; |
There was a problem hiding this comment.
| int iTalkingUI_TransparencyLevel = 255; | |
| int talkingUI_TransparencyLevel = 255; |
We dropped type hints in variable names for new code
| void LookConfig::on_qsbTransparencyLevel_valueChanged(int value) { | ||
| // Handle transparency level change | ||
| Q_UNUSED(value); | ||
| // You can add preview logic here if needed | ||
| } |
| void LookConfig::on_qcbTalkingUITransparent_stateChanged(int state) { | ||
| // Handle transparency checkbox state change | ||
| qsbTransparencyLevel->setEnabled(state == Qt::Checked); | ||
| } |
There was a problem hiding this comment.
In new code, we want to move away from implicit event connections by method name like this.
Please refactor this to use a plausible name like toggleTransparency or something and use the QObject::connect() method to explicitly connect the event.
Should look something like
QObject::connect(qcbTalkingUITransparent, QComboBox::stateChanged, this, LookConfig::toggleTransparency) (untested)
2 participants
Checks
Feature description
This implements list of changes so that "Talking UI" can act more like an actual overlay. The changes are:
See this video on how it should look. https://www.youtube.com/watch?v=FyiMCaXfKvg