[Publisher] Fix "Other" checking and unchecking behavior in Metric Settings #1639
Conversation
|
Hi @mxosman! Please test this changes and see if my fix solved the problem for you! I tried many different approaches, but it seems like only this particular solution is fixing it for me. Note: there could be already checked "other" dimensions (before this update) -- in that case they'll be checked without description since there is no way for FE to uncheck them from BE passively. I think this explains why "other" dimension is checked in the start of the second video from the task description (Prisons II agency). |
|
The thing I don't like about this solution is that the "Settings saved" toast popups twice when we hit "Save" button. After thinking some more about this and trying to find another solutions this is the only one that 100% solves the bug for me. |
|
Hi @nasaownsky - digging into it this now.
That makes sense - I'm OK with the behavior there.
Hmm, yeah - I'm not a fan of that b/c that means it's sending two requests each time. Any idea why it's sending two requests? Also separately, but potentially related - one thing I'm noticing too is that clicking the X button to close a modal sends a request, which might be contributing to some strange behavior. Do you know why that's happening? I thought the close modal code just updated local state. |
Ah - I see you already addressed this. |
| @@ -567,7 +566,8 @@ function DefinitionModalForm({ | |||
| {!isReadOnly && !hasNoSettingsAndNoContext && ( | |||
| <Button | |||
| label="Save" | |||
| onClick={() => { | |||
| onClick={async () => { | |||
| await handleOtherDimensionUpdate(); | |||
There was a problem hiding this comment.
Ah, we're calling saveMetricSettings twice with these two methods... What's the motivation behind awaiting this? Is there no way to consolidate these two save methods to avoid two redundant requests and still solve the issue?
There was a problem hiding this comment.
In the handleSaveSettings - I see we're referencing and looping through the currentSettings object - could we update that local state when folks update the Other Dimension so it's picked up by the saveMetricSettings in the handleSaveSettings function?
Oi - the DefinitionModalForm file is quite convoluted - thanks for swimming in this...
Description of the change
Fixed "Other" checking and unchecking behavior in Metric Settings page
Type of change
Related issues
closes #1621
Checklists
Development
This box MUST be checked by the submitter prior to merging:
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: