-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 "Card Width" property reset button. #5841
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. In addition to this PR, do we also need to clean up the value stored in local storage?
I don't think so. We can't identify or cleanup users who've tried to reset in the past. But the next time they press reset it should just work. |
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.
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.
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.
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:
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.