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

Reconsider how render_settings_field_multibox() works #36

Open
afercia opened this issue Jun 5, 2017 · 4 comments
Open

Reconsider how render_settings_field_multibox() works #36

afercia opened this issue Jun 5, 2017 · 4 comments

Comments

@afercia
Copy link
Contributor

afercia commented Jun 5, 2017

render_settings_field_multibox() perfectly works for a group of checkboxes sharing the same name attribute. That is, a multi-choice where users can check multiple values for the same group.

This works in HTML but it's arguable useful for settings since there's rarely (never?) a setting with multiple values.

Instead, the current group of checkboxes in core are all single settings with a unique name attribute. They're visually grouped because logically related. However, the don't use the same setting name.

How to render a group like the following one?

screen shot 2017-06-06 at 00 15 17

The only way right now would be a custom callback 😞

@afercia
Copy link
Contributor Author

afercia commented Jun 5, 2017

P.S. also, some checkboxes use the same value, for example the three ones above use:
1
open
open

and only two of them get printed out.

@felixarntz
Copy link
Contributor

I think the use-case for what the multibox field currently does should be left untouched. While there's no use-case in core (I think), it's a farely common functionality (often used in favor of select multiple).

For what you describe, in the context of the above three checkboxes, shouldn't this actually become a settings section ("Default article settings") with three individual checkboxes? This would match our idea of #5 as well.

@afercia
Copy link
Contributor Author

afercia commented Jun 7, 2017

Well actually they're not just three checkboxes, the Discussion page is full of single checkboxes grouped logically but used for different settings. Then maybe we should consider to have 2 separate functions:

  • the current multibox is actually for a group of checkboxes to be used for a single setting with multi-choice
  • a new one, to be used for logically grouped checkboxes that need to be within a fieldset but used for settings with a different setting name

Wort noting this second case, which as far as I see is the only one of the twos used in core, needs a different structure of the passed array because there are multiple settings names, so for each checkbox:

  • setting name
  • setting value
  • setting label

shouldn't this actually become a settings section ("Default article settings") with three individual checkboxes?
I think no, because we should also provide the ability to use groups of checkboxes that are single-choice (single setting) but logically related.

@felixarntz
Copy link
Contributor

felixarntz commented Jun 8, 2017

shouldn't this actually become a settings section ("Default article settings") with three individual checkboxes?

I think no, because we should also provide the ability to use groups of checkboxes that are single-choice (single setting) but logically related.

I think a settings section does exactly that, it groups fields that are logically related. We already have the functionality in place that we'd need for this.

a new one, to be used for logically grouped checkboxes that need to be within a fieldset but used for settings with a different setting name

Actually, this use-case would be a good argument for our idea to make settings sections actual fieldsets (see #14). If a settings section was a fieldset and contained the three checkboxes as single fields, it would have exactly the structure you're describing.

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

2 participants