Skip to content

Commit

Permalink
Fix "Card Width" property reset button. (#5841)
Browse files Browse the repository at this point in the history
The reset button for "Card Width" does not effectively reset the property. Pressing it resets it to the value that it was set to when the page was loaded. (So, for example, if the value is 535px on page load, pressing reset will reset it to 535px and not reset the property entirely).

The reasoning is:
* Pressing the reset button will remove the property from state, effectively making it undefined:
  * https://cs.opensource.google/tensorflow/tensorboard/+/master:tensorboard/webapp/metrics/store/metrics_reducers.ts;l=709;drc=2059b53ef0f6875dbaa8bca4362653c07d0069fd
* The PersistentSettingsDataSource does not update the field in local storage if the value is undefined:
  * https://cs.opensource.google/tensorflow/tensorboard/+/master:tensorboard/webapp/persistent_settings/_data_source/persistent_settings_data_source.ts;l=87;drc=54be1df836817d1a5a6378902eec23a9fe5cb40d
* And, so, the value will continue to be whatever was stored in local storage on page load. 

We fix the problem by setting the state value to null rather than removing it entirely. This means it will also be set to null in local storage and effectively reset the property.
  • Loading branch information
bmd3k committed Aug 4, 2022
1 parent 1648ec1 commit ec48bc1
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
6 changes: 4 additions & 2 deletions tensorboard/webapp/metrics/store/metrics_reducers.ts
Expand Up @@ -707,10 +707,12 @@ const reducer = createReducer(
};
}),
on(actions.metricsResetCardWidth, (state) => {
const {cardMinWidth, ...nextOverride} = state.settingOverrides;
return {
...state,
settingOverrides: nextOverride,
settingOverrides: {
...state.settingOverrides,
cardMinWidth: null,
},
};
}),
on(
Expand Down
4 changes: 1 addition & 3 deletions tensorboard/webapp/metrics/store/metrics_reducers_test.ts
Expand Up @@ -1201,9 +1201,7 @@ describe('metrics reducers', () => {
});
const nextState = reducers(prevState, actions.metricsResetCardWidth());
expect(nextState.settings.cardMinWidth).toBe(400);
expect(nextState.settingOverrides.hasOwnProperty('cardMinWidth')).toBe(
false
);
expect(nextState.settingOverrides.cardMinWidth).toBeNull();
});
});

Expand Down

0 comments on commit ec48bc1

Please sign in to comment.