-
Notifications
You must be signed in to change notification settings - Fork 18
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
Comments
Thanks @patrick-abc. Your code sample doesn’t seem to match what you’re asking for though – you’re not "add[ing] I wonder whether a better solution would be to have a 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(', ') );
} |
Yes it is intentional.
I think this case only make sense when a noneText is declared. But an additional setting is an option too. So I would suggest a setting like selectedPrefixText. |
I think ideally you would be using a @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. |
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:
To this:
The text was updated successfully, but these errors were encountered: