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

Numeric select options #1357

Closed
nilshoerrmann opened this issue Jan 15, 2019 · 15 comments
Closed

Numeric select options #1357

nilshoerrmann opened this issue Jan 15, 2019 · 15 comments

Comments

@nilshoerrmann
Copy link
Contributor

nilshoerrmann commented Jan 15, 2019

Describe the bug
When setting up select field options, Kirby ignores numeric keys:

options:
  300: '300: Multiple Choices'
  301: '301: Moved Permanently'

It doesn't matter if the keys are wrapped in quotes or not. Kirby will always use the text as value, storing 300: Multiple Choices instead of 300.

Expected behavior
Kirby should use the given keys. K2 seems to have handled this correctly when keys where wrapped in quotes.

Kirby Version
K3 Golden Master

@distantnative distantnative added the type: bug 🐛 Is a bug; fixes a bug label Jan 16, 2019
@bastianallgeier bastianallgeier added this to the 3.0.1 milestone Jan 21, 2019
@bastianallgeier
Copy link
Member

This is not really a Kirby bug, but a PHP stupidity. You can create an array with number strings as keys and it will automatically convert the strings to ints, which is super annoying.

There's a new feature in Kirby 3 for selects, radios and checkboxes, which makes it possible to declare options like this:

options: 
- Option A
- Option B
- Option C

which is often a lot more useful than repeating the value just to create the key, which would have been required in v2.

options: 
  Option A: Option A
  Option B: Option B
  Option C: Option C

But the only way to make this work is to check for integers as keys vs strings. That's where your setup fails. You can work around this by using the long form though:

options: 
  - value: 300
     text: Multiple Choices
  - value: 301
     text: Moved Permanently

@bastianallgeier bastianallgeier removed the type: bug 🐛 Is a bug; fixes a bug label Jan 21, 2019
@bastianallgeier bastianallgeier removed this from the 3.0.1 milestone Jan 21, 2019
@nilshoerrmann
Copy link
Contributor Author

Cool, thanks!
In this case the docs should be updated.

@hentrev
Copy link

hentrev commented Jan 23, 2019

Same problem here, although using the suggested solution with value and text, numerical values are not working. When saving the page in the panel, the value is not stored.

Did it work out for you, @nilshoerrmann ?

Version: 3.0.0

@texnixe
Copy link
Member

texnixe commented Jan 23, 2019

I just tested this and it works if you wrap the numbers in quotes

options: 
  - value: '300'
     text: Multiple Choices
  - value: '301'
     text: Moved Permanently

@hentrev
Copy link

hentrev commented Jan 24, 2019

Thanks @texnixe!
In this case, there is no need for the long form with value and text though.
Just using the numbers as strings in the short form is fine:

options: 
   - '1'
   - '2'
   - '3'

@texnixe
Copy link
Member

texnixe commented Jan 24, 2019

@hentrev Yes, but the whole issue was about displaying some meaningful text to accompany the numbers.

@texnixe
Copy link
Member

texnixe commented Jan 24, 2019

We can then probably close this?

@hentrev
Copy link

hentrev commented Jan 24, 2019

Sure, thanks for the quick support.

@nilshoerrmann
Copy link
Contributor Author

@texnixe: Will you add this to the docs?

@distantnative
Copy link
Member

Already has been :)

@nilshoerrmann
Copy link
Contributor Author

Perfect! Thanks a lot.

@neildaniels
Copy link
Contributor

I'm personally annoyed with the need to use the longhand form for this.

Would any of these two workarounds be considered?

Non-Zero Key:
If the first key in the array isn't 0, that seems like a giveaway that this should be treated as an associative array.

Con: If you actually do want to start at 0, this doesn't help you.

Padded String Keys:
If you prefix/suffix the numeric string keys with a space, PHP and Kirby will treat them as an associative array.
The only problem right now is that when the value is read (from the content file), the whitespace is ignored and the Panel "thinks" the saved value doesn't match any options. I suspect that a blueprint parsing change to trim whitespace from keys would avoid this issue.
(There should never be a case where whitespace is expected to be stored and retrieved.)

options:
  '300 ': '300: Multiple Choices'
  ' 301': '301: Moved Permanently'

Mixed Approach:
I think the non-zero key approach is the most straightforward. However, for the case that you want to start at 0, you could employ the padded string approach for the 0 case:

options:
  '0 ': 'Some value'
  '300': '300: Multiple Choices'
  '301': '301: Moved Permanently'

@vauvarin
Copy link

vauvarin commented Jul 8, 2019

Describe the bug
If we use just numbers as value and text, the value is written in the txt file but the corresponding radio button is not selected in the panel.

options:
      - value: '2'
        text: 2
      - value: '4'
        text: 4
      - value: '6'
        text: 6
      - value: '8'
        text: 8
      - value: '10'
        text: 10

Expected behavior
Radio button must be selected.

Kirby Version
3.2.0

@texnixe
Copy link
Member

texnixe commented Jul 8, 2019

@vauvarin If you wrap the text in quotes as well, it works.

        options:
          - value: '2'
            text: '2'
          - value: '4'
            text: '4'
          - value: '6'
            text: '6'
          - value: '8'
            text: '8'
          - value: '10'
            text: '10'

Or if you only use numbers like in your example, use the short form: #1357 (comment)

@beyond-interfaces
Copy link

i am trying to hook into an api and use IDs for countries and it fails on me, too:

country_id:
label: Country
type: select
options: api
api:
url: "{{ site.url }}/videonale/countries"
fetch: data
text: "{{ item.name }} ({{ item.iso2 }})"
value: "{{ item.country_id }}"

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

No branches or pull requests

8 participants