-
-
Notifications
You must be signed in to change notification settings - Fork 539
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
[5.x] Fix ButtonGroup not showing active state if value are numbers #10916
base: 5.x
Are you sure you want to change the base?
Conversation
Due to a change in 0b85403 the ButtonGroup fieldtype cannot display a button as active, if the key is a number. This commit will - make the ButtonGroup fieldtype work with number keys again - make the Width fieldtype work again, if the default options have been changed by the user, because changing the prefilled list will result in a mix of strings and integers - output labels consistently as strings
I appreciate all the work that went into this PR. Thank you.
Why can't we just make it a loose comparison in the JS instead?
Changing this to a loose comparison seems to solve the problem without any PHP changes needed.
It seems to work fine for me. It already uses a loose comparison.
|
4cb349e
to
ed47283
Compare
I thought about changing these types to ensure a better consistency. The PHP change actually affects the label type only, enforcing it to be a
Yea, it works :) |
This PR is ready for review, despite my long previous commit, just in case you got confused by it :) |
Due to a change in 0b85403 the ButtonGroup fieldtype cannot display a button as active, if the key is a number. This is because the HTML button uses strings for its value, but the PHP side will cast numeric values to numbers, which then will fail the
===
comparison.Also, if you change the default options for the
Width
fieldtype the resulting list will contain integer and string keys. It breaks theWidth
fieldtype to properly display the selected width, because it uses a strict comparison, too.Since all HTML form elements like select, input, button work with strings only, I thought it makes sense, to work with string values on the JS side only, too. The
Width
fieldtype needs to compare numbers, so it will cast all options to a number and ignore non-numeric values, which don't really make sense in this field.This change should not affect the augmented value of the fieldtypes, because during augmentation numeric values will be cast to an integer anyway.
This commit will
Fixes #10915.
Fixes #11029.