From 52d2f310eca02e8542b5db6c5e6cea31efb85d81 Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Mon, 20 Jun 2022 21:36:55 -0400 Subject: [PATCH 1/4] add condition to disable confirm button if no changes were made --- .../FeatureConfigurationFlyout.tsx | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx index c54846c29d..653cd12718 100644 --- a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx +++ b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx @@ -79,7 +79,9 @@ export class FeatureConfigurationFlyout extends React.Component< this.state = { items: [], newlySelectedFeatures: this.props.selectedFeatures, - newNumberOfContinuousFeatureBins: this.props.numberOfContinuousFeatureBins + newNumberOfContinuousFeatureBins: { + ...this.props.numberOfContinuousFeatureBins + } }; this.updateSelection(); } @@ -98,7 +100,12 @@ export class FeatureConfigurationFlyout extends React.Component< // At other times we can't update with props since they may be outdated compared to the state. if (this.props.isOpen && !prevProps.isOpen) { this.setState( - { newlySelectedFeatures: this.props.selectedFeatures }, + { + newlySelectedFeatures: this.props.selectedFeatures, + newNumberOfContinuousFeatureBins: { + ...this.props.numberOfContinuousFeatureBins + } + }, () => { this.updateSelection(); } @@ -181,6 +188,24 @@ export class FeatureConfigurationFlyout extends React.Component< private onRenderFooterContent = () => { const tooManyFeaturesSelected = this._selection.getSelectedCount() > 2; + const noChangesInSelectedFeatures = + // check that feature selection has not changed + this.props.selectedFeatures.length === + this.state.newlySelectedFeatures.length && + this.props.selectedFeatures.every( + (feature, featureIndex) => + this.state.newlySelectedFeatures[featureIndex] == feature + ) && + // check that number of continuous feature bins for the selected features has not changed + this.state.newlySelectedFeatures.every((_, featureIndex) => { + const newNumberOfBins = + this.state.newNumberOfContinuousFeatureBins[featureIndex] ?? + defaultNumberOfContinuousFeatureBins; + const prevNumberOfBins = + this.props.numberOfContinuousFeatureBins[featureIndex] ?? + defaultNumberOfContinuousFeatureBins; + return newNumberOfBins === prevNumberOfBins; + }); return ( {tooManyFeaturesSelected && ( @@ -195,7 +220,7 @@ export class FeatureConfigurationFlyout extends React.Component< Date: Mon, 20 Jun 2022 22:08:35 -0400 Subject: [PATCH 2/4] lintfix --- .../Controls/ModelOverview/FeatureConfigurationFlyout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx index 653cd12718..d6de13a3d7 100644 --- a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx +++ b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx @@ -194,7 +194,7 @@ export class FeatureConfigurationFlyout extends React.Component< this.state.newlySelectedFeatures.length && this.props.selectedFeatures.every( (feature, featureIndex) => - this.state.newlySelectedFeatures[featureIndex] == feature + this.state.newlySelectedFeatures[featureIndex] === feature ) && // check that number of continuous feature bins for the selected features has not changed this.state.newlySelectedFeatures.every((_, featureIndex) => { From da4940eb827c38a8d75127f73a8acabb3798cbb7 Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Tue, 21 Jun 2022 14:01:52 -0400 Subject: [PATCH 3/4] adjustments as recommended in pr --- .../FeatureConfigurationFlyout.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx index d6de13a3d7..712d6ce8cd 100644 --- a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx +++ b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx @@ -188,23 +188,24 @@ export class FeatureConfigurationFlyout extends React.Component< private onRenderFooterContent = () => { const tooManyFeaturesSelected = this._selection.getSelectedCount() > 2; - const noChangesInSelectedFeatures = // check that feature selection has not changed - this.props.selectedFeatures.length === - this.state.newlySelectedFeatures.length && - this.props.selectedFeatures.every( + const featureSelectionChanged = + this.props.selectedFeatures.length !== + this.state.newlySelectedFeatures.length || + this.props.selectedFeatures.some( (feature, featureIndex) => - this.state.newlySelectedFeatures[featureIndex] === feature - ) && + this.state.newlySelectedFeatures[featureIndex] !== feature + ); // check that number of continuous feature bins for the selected features has not changed - this.state.newlySelectedFeatures.every((_, featureIndex) => { + const continuousFeatureBinningChanged = + this.state.newlySelectedFeatures.some((_, featureIndex) => { const newNumberOfBins = this.state.newNumberOfContinuousFeatureBins[featureIndex] ?? defaultNumberOfContinuousFeatureBins; const prevNumberOfBins = this.props.numberOfContinuousFeatureBins[featureIndex] ?? defaultNumberOfContinuousFeatureBins; - return newNumberOfBins === prevNumberOfBins; + return newNumberOfBins !== prevNumberOfBins; }); return ( @@ -220,7 +221,7 @@ export class FeatureConfigurationFlyout extends React.Component< Date: Tue, 21 Jun 2022 14:02:12 -0400 Subject: [PATCH 4/4] lintfix --- .../ModelOverview/FeatureConfigurationFlyout.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx index 712d6ce8cd..d9212144e5 100644 --- a/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx +++ b/libs/model-assessment/src/lib/ModelAssessmentDashboard/Controls/ModelOverview/FeatureConfigurationFlyout.tsx @@ -188,16 +188,16 @@ export class FeatureConfigurationFlyout extends React.Component< private onRenderFooterContent = () => { const tooManyFeaturesSelected = this._selection.getSelectedCount() > 2; - // check that feature selection has not changed - const featureSelectionChanged = + // check that feature selection has not changed + const featureSelectionChanged = this.props.selectedFeatures.length !== this.state.newlySelectedFeatures.length || this.props.selectedFeatures.some( (feature, featureIndex) => this.state.newlySelectedFeatures[featureIndex] !== feature ); - // check that number of continuous feature bins for the selected features has not changed - const continuousFeatureBinningChanged = + // check that number of continuous feature bins for the selected features has not changed + const continuousFeatureBinningChanged = this.state.newlySelectedFeatures.some((_, featureIndex) => { const newNumberOfBins = this.state.newNumberOfContinuousFeatureBins[featureIndex] ?? @@ -221,7 +221,10 @@ export class FeatureConfigurationFlyout extends React.Component<