search

Kinark / Materialize-stepper

A little plugin that implements a stepper to Materializecss framework.
https://kinark.github.io/Materialize-stepper/
MIT License
227 stars 60 forks source link

Radio button lose status on step change #51

Closed lacc closed 6 years ago

lacc commented 6 years ago

Problem:

Radio buttons will lose its checked status on step change (either prev/next)

Issue:

As only 1 radio button is allowed to be checked with the same name, inserting the cloned radios into the HTML, will cause the others to be unchecked therefore losing the state.

Proposed solution:

In the getUnknownHeight method the name of the radios could be changed in the cloned element. Like in this PR: https://github.com/lacc/Materialize-stepper/pull/1

Kinark commented 6 years ago

It's a solution to the issue indeed, if you may, please create a PR in this repo. However, I wish to point three things:

1- All changes must be done in the develop branch. The master branch are just for releases. 2- Using the opportunity you gave us with the bug report, wouldn't it be good if we'd also change the ids of not the radios alone but of all the inputs? What do you think? I believe it'd be a good practice to prevent people having duplicated elements on the DOM (even tho it's just for a while). But in this case we'd have to change the fors of the labels as well. 3- If you may, please use this pattern of commenting:

// Capitalizing the phrases, adding a space after the double slashes and ending with a period.

Just for us to try to maintain everything organized.

Thank you :)

lacc commented 6 years ago

Thanks for the advice, I will create the PR on the developer branch, no problem.

Regarding the ID change, you right. Having elements with same ID is not optimal and brake the "rules". Although, I think IDs are still used as CSS selectors that could affect the height. In my opinion for this short time, multiple IDs may not a big problem, what do you think?

Thanks

Kinark commented 6 years ago

Yeah, you're right. It'd break some css rule indeed. Okay then, I think we're good to go with just the name attribute.

Thank you :)

I'll release a version soon with this fix, a fix for another reported bug (inverted validation function return check) and maybe some extra methods (like reset stepper, that's missing since the refactoring).