[Global Pins] Shows a confirmation dialog when disabling the global pin feature #6843
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roseayeon
changed the title
roseayeon
force-pushed
the
confirm-dialog
branch
from
e6343e2 to
6b28c50
Compare
roseayeon
force-pushed
the
confirm-dialog
branch
from
6b28c50 to
6ba778e
Compare
hoonji
approved these changes
Apr 30, 2024
| import {SavingPinsDialogModule} from './saving_pins_dialog_module'; | ||
| import {SavingPinsDialogComponent} from './saving_pins_dialog_component'; | ||
|
|
||
| describe('saving_pins_dialog', () => { |
There was a problem hiding this comment.
nit: Using underscores doesn't match the selector (or anything else) and can make the code harder to search for. Suggest: saving-pins-dialog or saving pins dialog.
(I know some other tests use underscores, but it doesn't seem to be consistent and I think we can adopt a better policy)
AnuarTB
pushed a commit
to AnuarTB/tensorboard
that referenced
this pull request
May 3, 2024
…in feature (tensorflow#6843) ## Motivation for features / changes Regarding tensorflow#6831 , this PR adds confirmation dialog" that appears when the user clears the checkbox to make the user aware that local storage is disappearing. ## Technical description of changes 0e63b07 created a `saving pins dialog` component 05e06f5 Created a `saving pins checkbox` component * The Save Pin checkbox prevents the default click and must be done after confirmation in the modal. So I added `MAT_CHECKBOX_DEFAULT_OPTIONS` (https://v15.material.angular.io/comComponents/checkbox/overview#click-action-config) to prevent the default click event. However, if I add this directly to the `settings view component`, all checkboxes will be disabled, including other unwanted ones. So, I created a separate checkbox for the Save Pin checkbox. Please let me know is there any other good way to do this. 6ba778e Make the dialog shows up when user clears the checkbox * Passes to `onEnableSavingPinsToggled` whether the checkbox is selected. * If the checkbox is selected and user clicks it, it means that the user disables the global pin, so a dialog box is displayed. * If the checkbox is not checked but is clicked, it means the user is trying to enable a global pin and no dialog box will be displayed. * `onEnableSavingPinsToggled` and `isSavingPinsEnabled` is passed to `saving-pins-checkbox` component. ## Screenshots of UI changes (or N/A)  ## Detailed steps to verify changes work correctly (as executed by you) unit test passes + TAP presubmit passes ## Alternate designs / implementations considered (or N/A)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for features / changes
Regarding #6831 , this PR adds confirmation dialog" that appears when the user clears the checkbox to make the user aware that local storage is disappearing.
Technical description of changes
0e63b07 created a
saving pins dialogcomponent05e06f5 Created a
saving pins checkboxcomponentMAT_CHECKBOX_DEFAULT_OPTIONS(https://v15.material.angular.io/comComponents/checkbox/overview#click-action-config) to prevent the default click event. However, if I add this directly to thesettings view component, all checkboxes will be disabled, including other unwanted ones. So, I created a separate checkbox for the Save Pin checkbox. Please let me know is there any other good way to do this.6ba778e Make the dialog shows up when user clears the checkbox
onEnableSavingPinsToggledwhether the checkbox is selected.onEnableSavingPinsToggledandisSavingPinsEnabledis passed tosaving-pins-checkboxcomponent.Screenshots of UI changes (or N/A)
Detailed steps to verify changes work correctly (as executed by you)
unit test passes + TAP presubmit passes
Alternate designs / implementations considered (or N/A)