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

Dialog > buttons option 'text' should be 'label' instead ? #2079

Open
pjfsilva opened this issue May 25, 2022 · 4 comments
Open

Dialog > buttons option 'text' should be 'label' instead ? #2079

pjfsilva opened this issue May 25, 2022 · 4 comments

Comments

@pjfsilva
Copy link

pjfsilva commented May 25, 2022

When we are setting the buttons for a dialog, the option to set the button text is 'text': https://api.jqueryui.com/dialog/#option-buttons

But on the Button Widget the option is called 'label'. https://api.jqueryui.com/button/#option-label

To make it consistent between the two places, shouldn't the option for the dialog be also called label?

Looking at the code for dialog.js it seems the dialog never sets buttonOptions.label thus making it impossible to use the right option on the dialog.

I understand this breaks compatibility but it's pretty easy to keep retrocompatibility and offer the label option on dialog. It also makes it easier to understand the dialog docs if we support all the button options over there. (most are supported but not all)

@mgol
Copy link
Member

mgol commented May 25, 2022

Thanks for the report. Does the issue you describe exist when jQuery UI 1.12.1 is used or only with jQuery UI 1.13.0 or newer?

@pjfsilva
Copy link
Author

@mgol
Copy link
Member

mgol commented Jun 1, 2022

Thanks for the report. Since the issue is already in 1.12, given limited team resources it's not likely to be fixed by the UI team; see the project status at https://blog.jqueryui.com/2021/10/jquery-maintainers-update-and-transition-jquery-ui-as-part-of-overall-modernization-efforts/. PRs are welcome if they're not too complex. However, we should preserve backwards compatibility here.

@fnagel what do you think? Should we accept such a change if submitted?

@fnagel
Copy link
Member

fnagel commented Jun 20, 2022

Sorry for the late response.

I agree that it should be possible to use the label and showLabel option when using the option buttons of the dialog widget. Actually only showLabel works but showText (as the docs say https://api.jqueryui.com/dialog/#option-buttons) does not. So the terminology is mixed up anyway. The source code looks like it was planned to remove the text option sometimes (see the Deprecated options" comments).

_createButtons: function() {

@mgol Yes for accepting a PR incl. support for dialog.buttons.label and dialog.buttons.showText for this from my side! We need to change the docs too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants