Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some issues found in the Test Module #5738

Merged
merged 3 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions QMLComponents/components/JASP/Controls/Button.qml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ JASPControl
readonly property alias control: control
property alias text: control.text
property alias label: control.text
property alias textFormat: control.textFormat
property alias iconSource: control.iconSource
readonly property alias pressed: control._pressed

Expand Down
3 changes: 2 additions & 1 deletion QMLComponents/components/JASP/Controls/CheckBox.qml
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ CheckBoxBase
property alias font: label.font
property alias fontInfo: label.fontInfo
property alias label: control.text
property alias labelTextFormat: label.textFormat
property alias checked: control.checked
property int textFormat: Text.AutoText
property bool childrenOnSameRow: false
property alias columns: childControlsArea.columns
property bool enableChildrenOnChecked: true
Expand Down Expand Up @@ -111,6 +111,7 @@ CheckBoxBase
font: jaspTheme.font
leftPadding: checkIndicator.width + control.spacing
verticalAlignment: Text.AlignVCenter
textFormat: checkBox.textFormat
}

background: Rectangle
Expand Down
2 changes: 2 additions & 0 deletions QMLComponents/components/JASP/Controls/ComboBox.qml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ ComboBoxBase
property alias value: comboBox.currentValue
property alias indexDefaultValue: comboBox.currentIndex
property alias fieldWidth: control.width
property int textFormat: Text.AutoText
property bool showVariableTypeIcon: containsVariables
property var enabledOptions: []
property bool setLabelAbove: false
Expand Down Expand Up @@ -87,6 +88,7 @@ ComboBoxBase
anchors.verticalCenter: parent.verticalCenter
color: enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
width: implicitWidth
textFormat: comboBox.textFormat
}
}

Expand Down
2 changes: 2 additions & 0 deletions QMLComponents/components/JASP/Controls/ExpanderButton.qml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ FocusScope
readonly property string iconsFolder : jaspTheme.iconPath
readonly property string expanderButtonIcon : "expander-arrow-up.png"
property alias columns : expanderArea.columns
property int textFormat : Text.AutoText

states: [
State
Expand Down Expand Up @@ -131,6 +132,7 @@ FocusScope
anchors.verticalCenter : parent.verticalCenter
font : jaspTheme.font
color : enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
textFormat : expanderWrapper.textFormat
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion QMLComponents/components/JASP/Controls/GroupBox.qml
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ GroupBoxBase
property int columnSpacing: jaspTheme.columnGroupSpacing
property int columns: 1
property bool indent: false
property bool alignFields: true
property bool alignFields: true
property alias label: label
property alias preferredWidth: contentArea.width
property int textFormat: Text.AutoText

property var _allAlignableFields: []
property bool _childrenConnected: false
Expand All @@ -55,6 +56,7 @@ GroupBoxBase
color: enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
font: jaspTheme.font
visible: groupBox.title ? true : false
textFormat: groupBox.textFormat

property int realHeight: visible ? implicitHeight : 0
property int realWidth: visible ? implicitWidth : 0
Expand Down
2 changes: 2 additions & 0 deletions QMLComponents/components/JASP/Controls/RadioButton.qml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ RadioButtonBase
property alias label: control.text
property alias checked: control.checked
property alias value: radioButton.name
property int textFormat: Text.AutoText
property bool childrenOnSameRow: false
property alias columns: childControlsArea.columns
property bool enableChildrenOnChecked: true
Expand Down Expand Up @@ -102,6 +103,7 @@ RadioButtonBase
leftPadding: radioIndicator.width + control.spacing
font: jaspTheme.font
color: enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
textFormat: radioButton.textFormat
}

background: Rectangle { color: "transparent" }
Expand Down
2 changes: 2 additions & 0 deletions QMLComponents/components/JASP/Controls/RadioButtonGroup.qml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ RadioButtonsGroupBase
property alias columns: contentArea.columns
property alias text: control.title
property int leftPadding: jaspTheme.groupContentPadding
property int textFormat: Text.AutoText

implicitWidth: radioButtonsOnSameRow
? contentArea.x + contentArea.implicitWidth
Expand All @@ -57,6 +58,7 @@ RadioButtonsGroupBase
verticalAlignment: Text.AlignVCenter
font: jaspTheme.font
color: enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
textFormat: control.textFormat
}


Expand Down
3 changes: 1 addition & 2 deletions QMLComponents/components/JASP/Controls/RectangularButton.qml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ Rectangle
{
id: filterButtonRoot



property alias textFormat: buttonText.textFormat
property string text: ""
property string toolTip: ""
property string textColor: "default"
Expand Down
3 changes: 3 additions & 0 deletions QMLComponents/components/JASP/Controls/TextField.qml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ TextInputBase
property alias control: control
property alias text: textField.label
property alias displayValue: control.text ///< In onEditingFinished this contains the "value" entered by the user
property int textFormat: Text.AutoText
property var lastValidValue: defaultValue
property int fieldWidth: jaspTheme.textFieldWidth
property int fieldHeight: 0
Expand Down Expand Up @@ -151,6 +152,7 @@ TextInputBase
color: enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
text: textField.label
width: implicitWidth
textFormat: textField.textFormat
}
}

Expand Down Expand Up @@ -279,6 +281,7 @@ TextInputBase
anchors.verticalCenter: parent.verticalCenter
color: enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
text: textField.afterLabel
textFormat: textField.textFormat
}
}
}
3 changes: 3 additions & 0 deletions QMLComponents/components/JASP/Controls/VariablesList.qml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ VariablesListBase
property alias itemRectangle : itemRectangle
property alias scrollBar : scrollBar
property alias itemTitle : itemTitle
property int textFormat : Text.AutoText
property string rowComponentTitle : ""
property string itemType : "variables"
property int dropMode : JASP.DropNone
Expand Down Expand Up @@ -146,6 +147,7 @@ VariablesListBase
height : title ? jaspTheme.variablesListTitle : 0
font : jaspTheme.font
color : enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
textFormat : variablesList.textFormat
}

Text
Expand All @@ -156,6 +158,7 @@ VariablesListBase
height : rowComponentTitle ? jaspTheme.variablesListTitle : 0
font : jaspTheme.font
color : enabled ? jaspTheme.textEnabled : jaspTheme.textDisabled
textFormat : variablesList.textFormat
}

Rectangle
Expand Down
19 changes: 17 additions & 2 deletions QMLComponents/controls/comboboxbase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ void ComboBoxBase::bindTo(const Json::Value& value)
index = int(std::distance(values.begin(), itr));
}
}
else if (!selectedValue.empty())
_lostValue = selectedValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Bedoel je "lost" value of "last" value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I called it lostValue because of the issue it solves: the dropdown is initialized with a value, but has no possible values (its model is empty). This is due to the fact that the VariablesList it depends on, gets initialized afterwards. In this case (when the dropdown has for values the levels of a VariablesList control), the dropdown has no way of guessing on which control it depends (values : varList.levels: the values are set with a list of strings, the levels, but it has no reference of the varList control self). This means that during the initialization of the form, VariablesList might be initialized after the dropdown.
Usually, if the dropdown is set with a non-existing value, it neglects this value (probably it comes from an old version of the analysis, and sets a warning), but in this case we want to keep this value until the form is completely initialized: if in the meantime, the dropdown gets a signal that its model is updated, it should try again to set this lostValue.
Maybe lostValue is not a good choice, but lastValue is definitly not he right name. This value should be used only during the initialization of the form. Some comments in the code would be nice also....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I replaced _lostValue by _unusedInitialValue. And add some comments


_setCurrentProperties(index);

Expand Down Expand Up @@ -133,7 +135,7 @@ Json::Value ComboBoxBase::createJson() const

bool ComboBoxBase::isJsonValid(const Json::Value &optionValue) const
{
return optionValue.type() == Json::stringValue || optionValue.type() == Json::arrayValue;
return optionValue.type() == Json::stringValue || optionValue.type() == Json::objectValue;
}

void ComboBoxBase::setUp()
Expand All @@ -151,8 +153,10 @@ void ComboBoxBase::setUp()
connect(this, &ComboBoxBase::currentValueChanged, [this] () { if (containsVariables()) checkLevelsConstraints(); } );

if (form())
{
connect(form(), &AnalysisForm::languageChanged, [this] () { _model->resetTermsFromSources(); } );

connect(form(), &AnalysisForm::analysisChanged, [this] () { _lostValue = ""; });
}
}

void ComboBoxBase::setUpModel()
Expand Down Expand Up @@ -180,6 +184,17 @@ void ComboBoxBase::termsChangedHandler()
{
auto itr = std::find(values.begin(), values.end(), fq(_currentValue));

if (!_lostValue.empty())
{
auto lostValueItr = std::find(values.begin(), values.end(), _lostValue);
if (lostValueItr != values.end())
{
itr = lostValueItr;
_orgValue = _lostValue;
_lostValue = "";
}
}

if (itr == values.end()) index = _getStartIndex();
else index = int(std::distance(values.begin(), itr));
}
Expand Down
1 change: 1 addition & 0 deletions QMLComponents/controls/comboboxbase.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ protected slots:
_currentColumnType,
_currentColumnRealType,
_currentColumnTypeIcon;
std::string _lostValue;
int _currentIndex = -1;
bool _fixedWidth = false;

Expand Down
2 changes: 1 addition & 1 deletion QMLComponents/controls/jasplistcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ bool JASPListControl::checkLevelsConstraints()

QStringList JASPListControl::levels() const
{
return initialized() ? model()->allLevels(model()->terms()) : QStringList();
return model() ? model()->allLevels(model()->terms()) : QStringList();
}

QStringList JASPListControl::allowedColumnsIcons() const
Expand Down
2 changes: 1 addition & 1 deletion QMLComponents/models/listmodelinteractionassigned.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ Terms ListModelInteractionAssigned::addTerms(const Terms& terms, int , const Row
Terms dropped;
if (availableModel())
dropped.setSortParent(availableModel()->allTerms());
dropped.set(terms);
dropped.set(checkTermsTypes(terms));

Terms newTerms = dropped.combineTerms(JASPControl::CombinationType::CombinationCross);

Expand Down
Loading