Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when we have two forms in the same page, the toggle checkbox doesn't work in the second form #1536

Closed
maysam opened this issue Mar 8, 2024 · 7 comments · Fixed by #1571
Closed
Assignees
Labels

Comments

@maysam
Copy link

maysam commented Mar 8, 2024

Description:

when we have two forms in the same page, the toggle checkbox doesn't work in the second form

Environment Details:

  • formBuilder Version: latest
  • Browser: chrome/safari
  • OS: macosx

Expected Behavior

checkbox to tggle

Actual Behavior

the checkbox in the second form doesn't toggle

Steps to Reproduce

put two forms in the same html page, both having toggle checkbox groups

Screenshot - (optional)

Referencing #24

@lucasnetau
Copy link
Collaborator

lucasnetau commented Mar 9, 2024

Can you please provide a reproduction and some screenshots? I cannot replicate.

Is this with formBuilder or formRender?

@maysam
Copy link
Author

maysam commented Mar 10, 2024

Hi @lucasnetau

It's actually renderer issue, I wasn't clear

https://codepen.io/maysam-the-scripter/pen/poBybPK

here is a demo, when you try to toggle the checkbox in the second form, it toggles the ones in the first form
it works fine when checkbox is not in toggle mode though

@lucasnetau
Copy link
Collaborator

You have the same ID for the control in both forms. So the selector is picking up the first control of that ID. Please adjust your form control ids so that you have valid HTML.

@maysam
Copy link
Author

maysam commented Mar 10, 2024

well this need to be localized, also this is only happening for the toggle, and other fields don't have this problem

in my application, I am rendering same form 4 times and this problem only happens when I turn on the toggle for checkboxes

@lucasnetau
Copy link
Collaborator

We can investigate if the selector can be reduced to target the expected control.

However it still holds that duplicate IDs in a single HTML page is invalid and you cannot expect anything to work. Giving a unique name in the formData JSON prior to render is a quick method (Array.map and append form-- to the name of each control field)

@lucasnetau
Copy link
Collaborator

This is the browser applying the checked flag when the label is clicked on. The label for the checkbox group includes the 'for' attribute and the duplicate ids mean the event is forwarded to the first element of that id. The for attribute can be removed for toggles however there are other javascript errors seen in formBuilder when you have two checkbox fields with the same name.

@kevinchappell Removing the for attribute for toggles is simple enough, generates valid HTML and the toggles will work. However I don't believe we should be fixing any other duplicate ID issues since it is invalid markup.

@kevinchappell kevinchappell self-assigned this Apr 6, 2024
@lucasnetau
Copy link
Collaborator

@kevinchappell this is the quick patch I tested, since the label wraps the form element in the kc-toggle the for attribute isn't required anymore

diff --git a/src/js/control/select.js b/src/js/control/select.js
index 9de4191..163fa98 100644
--- a/src/js/control/select.js
+++ b/src/js/control/select.js
@@ -93,6 +93,7 @@ export default class controlSelect extends control {
           const labelAttrs = { for: optionAttrs.id }
           let output = [input, this.markup('label', labelContents, labelAttrs)]
           if (toggle) {
+            delete labelAttrs.for
             labelAttrs.className = 'kc-toggle'
             labelContents.unshift(input, this.markup('span'))
             output = this.markup('label', labelContents, labelAttrs)

kevinchappell added a commit that referenced this issue Jul 4, 2024
Fix GH-1536 kc-toggle doesn't work with duplicated ids
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants