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

In the radio buttons, the option()'s two arguments are reversed. #4948

Closed
1 of 17 tasks
dongjoon9331 opened this issue Dec 14, 2020 · 7 comments
Closed
1 of 17 tasks

In the radio buttons, the option()'s two arguments are reversed. #4948

dongjoon9331 opened this issue Dec 14, 2020 · 7 comments
Labels

Comments

@dongjoon9331
Copy link

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Details about the bug:

In the radio buttons, the parameters of the option() are applied interchanged.
This error is occurring in the latest version of p5.js.
To see what the error occurs, refer to the url below.
https://p5js.org/ko/reference/#/p5/createRadio
In this example, the amount is displayed where the name of the food should appear.

@welcome
Copy link

welcome bot commented Dec 14, 2020

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

@limzykenneth
Copy link
Member

Hi @dongjoon9331, to clarify, do you mean the behaviour in the library isn't what you expected or the documentation example isn't what you expected?

At further down the page the documentation for the .option() method it says:

.option(value, [label]) can be used to create a new option for the element. If an option with a value already exists, it will be returned. Optionally, a label can be provided as second argument for the option.

The label is what is displayed on the page, while value is the form value of the element.

Basically from what I see the behaviour of the code is as I expected, unless you mean the value and label for the second example should be switched around?

@dongjoon9331
Copy link
Author

dongjoon9331 commented Dec 15, 2020

In previous versions of p5.js, it worked as .option([label], value).
By the way, it seems to work as .option(value, [label]) in the latest version.

In the second example, the name of the food is printed as the price. This obviously means that the order of the .option() arguments has changed.

If you replace the p5.js library without knowing this, you will surely get an error in your program.

@dongjoon9331
Copy link
Author

Hi @dongjoon9331, to clarify, do you mean the behaviour in the library isn't what you expected or the documentation example isn't what you expected?
At further down the page the documentation for the .option() method it says:

.option(value, [label]) can be used to create a new option for the element. If an option with a value already exists, it will be returned. Optionally, a label can be provided as second argument for the option.

The label is what is displayed on the page, while value is the form value of the element.
Basically from what I see the behaviour of the code is as I expected, unless you mean the value and label for the second example should be switched around?

The problem is that it was used as .option ([label], value) in previous versions.
If it is programmed according to the previous version of the library, just replacing p5.dom.js will destroy all existing programs.

@limzykenneth
Copy link
Member

limzykenneth commented Dec 15, 2020

The changes to createRadio was made in #4430 and the breaking changes was not noticed back then. @lmccart Technically this is a breaking change introduced after 1.0.0 (changes made between 1.0.0 and 1.1.3 releases), the current API matches the rest of the library in terms of consistency but it does mean sketches written between 1.0.0 (29 Feb 2020) and 1.1.3 (22 Jul 2020) would be affected.

@dongjoon9331 For sketches before then, especially before p5.dom.js is merged into the core library, the p5.js version is differentiated by a major version so breaking changes can occur so no guaranteed can be made about code written for p5.js 0.x versions to be fully compatible with code written for p5.js 1.x versions.

@dongjoon9331
Copy link
Author

@limzykenneth Okay, I see.
However, it's really difficult to modify a lot of my code in order to change the p5.js library.
Thank you for your reply.

@lmccart
Copy link
Member

lmccart commented Dec 18, 2020

@limzykenneth thanks for the clarification. while it is a breaking change, it looks like it was made to support the optional second argument, which makes sense to me. I'd suggest we keep as is, but update the example in the documentation to make more sense (flip the strings and numbers in the option() calls)

nanselmo added a commit to nanselmo/p5.js that referenced this issue Jan 20, 2022
Updating example and documentation related to processing#4948.
The label and the value parameters are swapped in the latest implementation of radio() (processing#4430)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants