Fix: Added unique group names for radio buttions #7162
Conversation
|
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page! |
src/dom/dom.js
Outdated
| @@ -1738,6 +1723,10 @@ p5.prototype.createRadio = function(...args) { | |||
| return self; | |||
| }; | |||
|
|
|||
| // Initialize counter for unique radio group names | |||
| p5.prototype.createRadio.counter = 0; | |||
There was a problem hiding this comment.
I wonder if p5's prototype is a good place to keep this counter, although I am not much aware of the code base to provide an alternative.
There was a problem hiding this comment.
thank you for the review , please where is the prototype class . I will be happy to keep the counter there. I am still learning the structure of the codebase.
There was a problem hiding this comment.
I see that we are not adding anything new to the prototype of p5 ... we are adding to createRadio which is already there, so I guess this can be OK, you can ignore my previous comment
There was a problem hiding this comment.
I'm not sure attaching to createRadio as a property is a good idea here. If it is just something that is kept tracked of internally, it can just be a scoped variable, ie. let counter = 0.
There was a problem hiding this comment.
When I use let counter = 0 within the function’s scope, the counter resets with each call to createRadio, and causes duplicate names like radioOption_0 for multiple radio groups. When I attach the counter to p5.prototype.createRadio, each group gets a unique identifier ( radioOption_1, radioOption_2), avoiding conflicts between radio groups.
There was a problem hiding this comment.
The let statement should be outside of the function scope, it can be in the file scope as the bundler won't leak file level local variable to the larger environment.
There was a problem hiding this comment.
You are right. I have made the changes with the global counter.
Forchapeatl
changed the title
|
I see many of the existing code comments are removed, is there a reason for that? |
I have placed back the comments |
|
Looks good. Thanks! |
This PR ensures unique names for each radio button group in createRadio function
Resolves #6763
Resolves #6763
Changes:
self._nameAssignmentScreenshots of the change:
Apex_1723102309099.mp4
PR Checklist
npm run lintpasses