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

Prevent duplicate attributes #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JeffBuckles
Copy link

Non-array control attributes are concatenated with a space delimiter. This caused me some problems where I would end up with multiple occurrences of some attribute values. Added some checking to confirm the attribute value is not already present before adding it.

There may be consequences or implications of doing this that I'm not aware of, so please consider carefully.

Also noticed my braces style does not match your standard (I use braces even if only one line of code in the block). Let me know if you want me to fix it.

Thanks,

@stefangabos
Copy link
Owner

Can you please give me a real-life example where this is causing problems? I've used this only for adding multiple classes to an element, but having the same class added multiple times is not an issue so I've never considered adding this check. Other attributes usually accept a single value...

@JeffBuckles
Copy link
Author

I had to think about this for a while because I made the change in my
local copy back in February, and only now got around to reviewing how
to do pull requests in GitHub...

This is a consequence of how I am processing the form after validation
(the form is always re-displayed for further editing).

Various conditions may cause controls to be disabled or re-enabled.
Because multiple conditions were checked, it sometimes caused the
"disabled" class to be added more than once, for example. This meant
that to re-enable the control I had to check each time for multiple
occurrences of 'disabled' in the class attribute.

Rather than fixing my code to check for duplicates everywhere I touched
the attributes (or re-write the whole thing with a more sensible
structure), it was more convenient to prevent the duplication from
occurring at all.

I'm happy to keep this as a local patch if it's not an issue
for anyone else.

Thanks and Best Regards,
-- Jeff

On 5/26/2016 1:54 AM, Stefan Gabos wrote:

Can you please give me a real-life example where this is causing
problems? I've used this only for adding multiple classes to an element,
but having the same class added multiple times is not an issue so I've
never considered adding this check. Other attributes usually accept a
single value...


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#28 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants