fix: don't show this again toggle bug #460

[JoaoOliveira85]

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

element to ensure that the toggle state is updated when clicked.

Screenshots (if necessary)

Additional context

Discord username (if different from GitHub):

[Benjozork]

Can you add a changelog entry to changelog.yml?

[JoaoOliveira85]

Can you add a changelog entry to changelog.yml?

Excuse me but could you clarify? The changelog.yaml file seems to be auto generated. Do you want me to add a Unreleased line or something similar?

#   - name: Unreleased
#    changes:
#      - title: Fixed "show this again" toggle button on installer
#        categories: [UI]
#        authors: [JoaoOliveira85]

[FoxtrotSierra6829]

The changelog file is not autogenerated. You have to write the content yourself. Just augment the version by one

[JoaoOliveira85]

The changelog file is not autogenerated. You have to write the content yourself. Just augment the version by one

Gotcha! Updated :)

[FoxtrotSierra6829]

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.

[JoaoOliveira85]

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.