fix: don't show this again toggle bug #460 #476
Conversation
Excuse me but could you clarify? The changelog.yaml file seems to be auto generated. Do you want me to add a |
|
The changelog file is not autogenerated. You have to write the content yourself. Just augment the version by one |
Gotcha! Updated :) |
There was a problem hiding this comment.
Unfortunately after testing and investigating I have to tell you this PR does not fix the issue, it just inverts it. The reason why clicking on the toggle itself didn't work in the past is that the click event was actually executed twice, once by the toggle and once by its parent. Now adding an onClick event to the parent's parent inverts this relationship again, so that clicking on the toggle actually triggers it 3 times, but everything around it will trigger 2 times and as such not work.
The solution is to let the onToggle() event of the Toggle that is triggered within the CompactYesNoOptionToggle and YesNoOptionToggle components do nothing.
Co-authored-by: Florian Scheuner <[email protected]>
|
Good catch on that analysis. Your suggested solution does work but perhaps, instead of changing the expected behavior from the toggle it'd make more sense to use preventPropagation(). I'll try to get some time to take a look at it since, currently, the onClick attribute is only designed to handle a boolean inversion and doesn't pass the event along so I don't have access to the event methods to use preventPropagation. Since I'm not very familiar with the codebase I don't want to break anything. :) Thanks again for your feedback. |
Fixes #460
Summary of Changes
The "Don't Show This Again" toggle wasn't working correctly because the click event wasn't being captured. To fix this, I added an onClick to the dialog's
Screenshots (if necessary)
Additional context
Discord username (if different from GitHub):