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

Append allText to selected Text in button #37

Open
patrick-abc opened this issue Nov 29, 2022 · 3 comments
Open

Append allText to selected Text in button #37

patrick-abc opened this issue Nov 29, 2022 · 3 comments

Comments

@patrick-abc
Copy link

When defined a allText option, then select the options, it would be nice to add the allText before.

Proposal to change the updateButtonContents function.

Change this:

if (selected.length == 0) {
  this.$button.text( this.settings['noneText'] );
} else if ( (selected.length === options.length) && this.settings['allText']) {
  this.$button.text( this.settings['allText'] );
} else {
  this.$button.text( selected.join(', ') );
}

To this:

if (selected.length == 0) {
  this.$button.text( this.settings['noneText'] );
} else if ( (selected.length === options.length) && this.settings['allText']) {
  this.$button.text( this.settings['allText'] );
} else if (this.settings['noneText']){
  this.$button.text( this.settings['noneText'] + ': ' + selected.join(', ') );
} else {
  this.$button.text( selected.join(', ') );
}
@zarino
Copy link
Member

zarino commented Nov 29, 2022

Thanks @patrick-abc. Your code sample doesn’t seem to match what you’re asking for though – you’re not "add[ing] allText before" the list of selected options, you’re adding noneText instead. Is that intentional?

I wonder whether a better solution would be to have a someText option, to go alongside allText and noneText, and then prefix that (if it’s been set). Something like…

if (selected.length == 0) {
  this.$button.text( this.settings['noneText'] );
} else if ( (selected.length === options.length) && this.settings['allText']) {
  this.$button.text( this.settings['allText'] );
} else if (this.settings['someText']){
  this.$button.text( this.settings['someText'] + ': ' + selected.join(', ') );
} else {
  this.$button.text( selected.join(', ') );
}

@patrick-abc
Copy link
Author

Yes it is intentional.
Example: I declare a select with noneText "Fruits".

  1. I click into "Fruits" the multiselect was open and show "Apple", "Pear", "Pineapple"
  2. I will select "Apple" and "Pear" and close the multiselect
  3. Now it displays "Apple, Pear" in the select
  4. With the code above from me, it would display: "Fruits: Apple, Pear"

I think this case only make sense when a noneText is declared.
So the idea is to use the select without a label.

But an additional setting is an option too. So I would suggest a setting like selectedPrefixText.
Then I could add "Fruits :" for selectedPrefixText in my example.

@zarino
Copy link
Member

zarino commented Nov 29, 2022

I think ideally you would be using a <label> element outside the <select>, rather than using the noneText as a sort of placeholder / internal label.

@dracos any thoughts? I’m wonder whether maybe we could refactor these lines out into a function (which is passed a list of selected options, plus all the settings) and then we could allow people to define their own callback function, to replace that, as part of the config.

In the meantime @patrick-abc you’re welcome to fork this repo, make the change yourself, and submit a pull request back.

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