Restore user preferences (#456)
### Description of the changes: **1) Do not set units to default values if they already have valid values** This fixes the actual issue. `UnitConverter::InitializeSelectedUnits()` ( this function resets all units to their default units if available for the current category ) gets called after `UnitConverterViewModel::RestoreUserPreferences()` ( this function restores user preferences ). So Calculator has been restoring saved values, and then overriding the restored values with default values. I modified `InitializeSelectedUnits()` so that we only initialize units only when they are not already set to valid units for the current category. **2) Removed `m_isFirstTime`** I noticed that we are calling `RestoreUserPreferences()` twice when Calculator starts up, and the function is restoring the same value both times The below happens when Calculator starts up 1) On startup, in `UnitConverterViewModel::InitializeView()`, `RestoreUserPreferences()` is called. 2) `RestoreUserPreferences()` in turn triggers `OnUnitChanged()` 3) During the first call to `OnUnitChanged()`, m_IsFirstTime is `True`, so we call `RestoreUserPreferences()` again while also setting `m_IsFirstTime` to `False`. 4) `RestoreUserPreference()` again triggers `OnUnitChanged()` 5) During the second call to `OnUnitChanged()`, m_IsFirstTime is `False`, so we call `SaveUserPreferences()` I think we should only call `SaveUserPreferences()` inside `OnUnitChanged()` since we already restored user preferences during view initialization. I can't really think of a reason to restore units after view has been initialized. This led me to just delete `m_isFirstTime`. ### How changes were validated: Manually tested that units and the current category are properly selected when you quit and start Calculator.   ## Fixes #445.
This commit is contained in:
		
				
					committed by
					
						
						Howard Wolosky
					
				
			
			
				
	
			
			
			
						parent
						
							8106691d7c
						
					
				
				
					commit
					af8322617f
				
			@@ -791,17 +791,27 @@ void UnitConverter::InitializeSelectedUnits()
 | 
				
			|||||||
    vector<Unit> curUnits = itr->second;
 | 
					    vector<Unit> curUnits = itr->second;
 | 
				
			||||||
    if (!curUnits.empty())
 | 
					    if (!curUnits.empty())
 | 
				
			||||||
    {
 | 
					    {
 | 
				
			||||||
 | 
					        // Units may already have been initialized through UnitConverter::RestoreUserPreferences().
 | 
				
			||||||
 | 
					        // Check if they have been, and if so, do not override restored units.
 | 
				
			||||||
 | 
					        bool isFromUnitValid = m_fromType != EMPTY_UNIT && find(curUnits.begin(), curUnits.end(), m_fromType) != curUnits.end();
 | 
				
			||||||
 | 
					        bool isToUnitValid = m_toType != EMPTY_UNIT && find(curUnits.begin(), curUnits.end(), m_toType) != curUnits.end();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        if (isFromUnitValid && isToUnitValid)
 | 
				
			||||||
 | 
					        {
 | 
				
			||||||
 | 
					            return;
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        bool conversionSourceSet = false;
 | 
					        bool conversionSourceSet = false;
 | 
				
			||||||
        bool conversionTargetSet = false;
 | 
					        bool conversionTargetSet = false;
 | 
				
			||||||
        for (const Unit& cur : curUnits)
 | 
					        for (const Unit& cur : curUnits)
 | 
				
			||||||
        {
 | 
					        {
 | 
				
			||||||
            if (!conversionSourceSet && cur.isConversionSource)
 | 
					            if (!conversionSourceSet && cur.isConversionSource && !isFromUnitValid)
 | 
				
			||||||
            {
 | 
					            {
 | 
				
			||||||
                m_fromType = cur;
 | 
					                m_fromType = cur;
 | 
				
			||||||
                conversionSourceSet = true;
 | 
					                conversionSourceSet = true;
 | 
				
			||||||
            }
 | 
					            }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
            if (!conversionTargetSet && cur.isConversionTarget)
 | 
					            if (!conversionTargetSet && cur.isConversionTarget && !isToUnitValid)
 | 
				
			||||||
            {
 | 
					            {
 | 
				
			||||||
                m_toType = cur;
 | 
					                m_toType = cur;
 | 
				
			||||||
                conversionTargetSet = true;
 | 
					                conversionTargetSet = true;
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -148,7 +148,6 @@ UnitConverterViewModel::UnitConverterViewModel(const shared_ptr<UCM::IUnitConver
 | 
				
			|||||||
    Unit2AutomationName = m_localizedOutputUnitName;
 | 
					    Unit2AutomationName = m_localizedOutputUnitName;
 | 
				
			||||||
    IsDecimalEnabled = true;
 | 
					    IsDecimalEnabled = true;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    m_IsFirstTime = true;
 | 
					 | 
				
			||||||
    m_model->Initialize();
 | 
					    m_model->Initialize();
 | 
				
			||||||
    PopulateData();
 | 
					    PopulateData();
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
@@ -156,7 +155,6 @@ UnitConverterViewModel::UnitConverterViewModel(const shared_ptr<UCM::IUnitConver
 | 
				
			|||||||
void UnitConverterViewModel::ResetView()
 | 
					void UnitConverterViewModel::ResetView()
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
    m_model->SendCommand(UCM::Command::Reset);
 | 
					    m_model->SendCommand(UCM::Command::Reset);
 | 
				
			||||||
    m_IsFirstTime = true;
 | 
					 | 
				
			||||||
    OnCategoryChanged(nullptr);
 | 
					    OnCategoryChanged(nullptr);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -239,15 +237,8 @@ void UnitConverterViewModel::OnUnitChanged(Object ^ parameter)
 | 
				
			|||||||
        // End timer to show results immediately
 | 
					        // End timer to show results immediately
 | 
				
			||||||
        m_supplementaryResultsTimer->Cancel();
 | 
					        m_supplementaryResultsTimer->Cancel();
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
    if (!m_IsFirstTime)
 | 
					
 | 
				
			||||||
    {
 | 
					 | 
				
			||||||
    SaveUserPreferences();
 | 
					    SaveUserPreferences();
 | 
				
			||||||
    }
 | 
					 | 
				
			||||||
    else
 | 
					 | 
				
			||||||
    {
 | 
					 | 
				
			||||||
        RestoreUserPreferences();
 | 
					 | 
				
			||||||
        m_IsFirstTime = false;
 | 
					 | 
				
			||||||
    }
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
void UnitConverterViewModel::OnSwitchActive(Platform::Object ^ unused)
 | 
					void UnitConverterViewModel::OnSwitchActive(Platform::Object ^ unused)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -311,8 +311,6 @@ namespace CalculatorApp
 | 
				
			|||||||
            std::wstring m_valueFromUnlocalized;
 | 
					            std::wstring m_valueFromUnlocalized;
 | 
				
			||||||
            std::wstring m_valueToUnlocalized;
 | 
					            std::wstring m_valueToUnlocalized;
 | 
				
			||||||
            bool m_relocalizeStringOnSwitch;
 | 
					            bool m_relocalizeStringOnSwitch;
 | 
				
			||||||
            // For Saving the User Preferences only if the Unit converter ViewModel is initialised for the first time
 | 
					 | 
				
			||||||
            bool m_IsFirstTime;
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
            Platform::String ^ m_localizedValueFromFormat;
 | 
					            Platform::String ^ m_localizedValueFromFormat;
 | 
				
			||||||
            Platform::String ^ m_localizedValueFromDecimalFormat;
 | 
					            Platform::String ^ m_localizedValueFromDecimalFormat;
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user