add option to delete table and schema immediately #14736
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14736 +/- ##
============================================
+ Coverage 61.75% 63.82% +2.07%
- Complexity 207 1607 +1400
============================================
Files 2436 2703 +267
Lines 133233 150729 +17496
Branches 20636 23289 +2653
============================================
+ Hits 82274 96206 +13932
- Misses 44911 47326 +2415
- Partials 6048 7197 +1149
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Thanks! This is super useful! @jayeshchoudhary Can you help take a look? |
| // This is quite hacky, but it's the only way to get this to work with the dialog. | ||
| // The useState variables are simply for the dialog box to know what to render in | ||
| // the checkbox fields. The actual state of the checkboxes is stored in the refs. | ||
| // The refs are then used to determine how we delete the table. If you try to use | ||
| // the state variables in this class, you will not be able to get their latest values. | ||
| const [dialogCheckboxes, setDialogCheckboxes] = useState({ | ||
| deleteImmediately: false, | ||
| deleteSchema: false | ||
| }); | ||
| const deleteImmediatelyRef = useRef(false); | ||
| const deleteSchemaRef = useRef(false); | ||
|
|
There was a problem hiding this comment.
if the state can be used to render checkbox value then it can be used at other places as well.
do you know the reason of this behaviour? Iet me know if you want me to debug. I think its some react rendering issue.
There was a problem hiding this comment.
I don't know react well enough, but I spent days on this with a coworker as well as multiple LLM tools, and this is the only thing that reliably worked. I think the issue is the very strange lifecycle of state variables and functions.
- "show dialog" state variable is in TenantDetails
- checkbox definition is in TenantDetails
- checkbox state is in TenantDetails
- the actual modal is in Confirm
- the children are rendered in Confirm
- the callbacks are defined in TenantDetails but activated in Confirm
Reusing the state variables from TenantDetails and rendering them in Confirm, I only got 2 bad things working
- the checkbox updates, but the callback doesn't see the latest value until the second time it's called
- the callback sees the latest value, but the checkbox doesn't actually visually update
This leads me to believe that the way we're creating + rendering the modal does not allow for a single state variable to work since there's no possible update order for every component to do the right thing.
There was a problem hiding this comment.
got it, thanks for the info. Let me try debugging.
Will update you here in a day or two. if not this solution looks good enough.
This is a small
UIquality of lifefeatureI tested this locally with a quickstart:
how the modal looks
deleting both the table and schema
