[dashboard] Add alert when user deleting root level tab #5771
Conversation
graceguo-supercat
force-pushed
the
gg-DeleteTabAlert
branch
from
4eea0b4
to
a158b7e
Compare
graceguo-supercat
changed the title
|
@graceguo-supercat great change! A couple of aesthetic comments:
""" Deleting a tab will remove all content within it. You may still reverse this action with the undo button (...+ z) until you save your changes. |
Codecov Report
@@ Coverage Diff @@
## master #5771 +/- ##
==========================================
+ Coverage 63.78% 63.79% +0.01%
==========================================
Files 364 365 +1
Lines 23111 23138 +27
Branches 2587 2591 +4
==========================================
+ Hits 14741 14762 +21
- Misses 8355 8361 +6
Partials 15 15
Continue to review full report at Codecov.
|
| <div className="delete-component-modal"> | ||
| <div> | ||
| <p /> | ||
| <b>Warning:</b> If you delete this dashboard tab all content |
There was a problem hiding this comment.
see my comments about updated messaging
| className: PropTypes.string, | ||
| label: PropTypes.string, | ||
| }; | ||
|
|
||
| const defaultProps = { | ||
| className: null, | ||
| label: null, | ||
| onClick: () => true, |
There was a problem hiding this comment.
hmm this is a little strange, but I guess is okay. Is this preferred so you don't have to set styles on a simple <div className="fa fa-trash" />?
There was a problem hiding this comment.
i wanted to re-use IconButton component to save some styling work. I only need that icon as a trigger to show modal, and ModalTrigger will bind onClick event listener.
but right now i feel should not change IconButton original behavior, which always required a callback. So i just use regular <div> as trigger component.
| @@ -36,19 +37,19 @@ class WithPopoverMenu extends React.PureComponent { | |||
|
|
|||
| componentWillReceiveProps(nextProps) { | |||
| if (nextProps.editMode && nextProps.isFocused && !this.state.isFocused) { | |||
| document.addEventListener('click', this.handleClick, true); | |||
There was a problem hiding this comment.
yeah still can't remember why I added these 🤷♀️
| margin: 40px 0 12px 0; | ||
|
|
||
| .btn { | ||
| margin-right: 24px; |
There was a problem hiding this comment.
could we cut this in half?
There was a problem hiding this comment.
i fixed padding, message text, and a few modal CSS. please see the updated attachment.
|
@graceguo-supercat here's more details about my margin suggestions, let me know if it doesn't make sense. basically I think we should have consistent "pink" sized padding everywhere.
|
|
Thanks for your aestheic suggestions @williaster! @graceguo-supercat could we remove the The command |
graceguo-supercat
force-pushed
the
gg-DeleteTabAlert
branch
from
a158b7e
to
3cf7f60
Compare
|
i updated code per comments, and attached new screenshot. please take a look. Thanks @elibrumbaugh @williaster |
graceguo-supercat
force-pushed
the
gg-DeleteTabAlert
branch
from
3cf7f60
to
92199de
Compare
graceguo-supercat
force-pushed
the
gg-DeleteTabAlert
branch
from
92199de
to
8f6be30
Compare
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
(cherry picked from commit 0c98ecb)
mistercrunch
added
🏷️ bot
one user reported that while she was editing dashboard, she accidentally deleted a root-level tab, but she didn't notice the action. She saved all the changes, but later she found the loss is big.
So we want to add a alert when user deleting a root level tab. The tab will be removed after user confirm. The removed tab can still be recovered from undo.
Step 1
Step 2
before: tab is deleted.
after: