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

Document parameters of p5.Color constructor #6006

Closed
1 of 17 tasks
Pomax opened this issue Feb 5, 2023 · 18 comments · Fixed by #6047
Closed
1 of 17 tasks

Document parameters of p5.Color constructor #6006

Pomax opened this issue Feb 5, 2023 · 18 comments · Fixed by #6047

Comments

@Pomax
Copy link

Pomax commented Feb 5, 2023

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build Process
  • Unit Testing
  • Internalization
  • Friendly Errors
  • Other (specify if possible)

p5.js version

1.5.0 (cloudflare CDN)

Web browser and version

all of them

Operating System

n/a

Steps to reproduce this

Steps:

<!DOCTYPE html>

<html lang="en">
<head>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.5.0/p5.js"></script>
</head>
<body>
  <script>
    p5.Color(0)
  </script>
</body>
</html>

Yields the following error:

Uncaught TypeError: this._storeModeAndMaxes is not a function
    Color https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.5.0/p5.js:53391
    <anonymous> http://localhost:8000/index.html:9
[p5.js:53391:16](https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.5.0/p5.js)
    Color https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.5.0/p5.js:53391
    <anonymous> http://localhost:8000/index.html:9
@Pomax Pomax added the Bug label Feb 5, 2023
@welcome
Copy link

welcome bot commented Feb 5, 2023

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@davepagurek
Copy link
Contributor

davepagurek commented Feb 5, 2023

Does this still happen if you call this in setup and use the color method to construct a p5.Color object? Generally, no p5 types can be used until setup has been run, and color() is the documented way to create p5 colors.

Also: hi, thanks for bezierjs!

@Pomax
Copy link
Author

Pomax commented Feb 6, 2023

haha, you're welcome =D

It does not. I ran into this while running someone else's code in a different project, so I guess I'll report back that they may want to fixer-upper their code ;)

@davepagurek
Copy link
Contributor

Sounds good! I'm going to close this issue for organizational purposes but feel free to discuss more if anything new comes up and I can always reopen it 🙂

@martinfalk
Copy link

Just came across this myself. Calling p5.Color() in setup() or after does not make a difference. Still throws a TypeError.
See minimal (non-)working example

Btw. the docs of p5.Color do not state that one should use color() instead ;-)

@davepagurek
Copy link
Contributor

Thanks @martinfalk, makes sense, I can reopen this as a documentation issue on the p5.Color class. The constructor has two required parameters, but aren't mentioned in the docs.

@davepagurek davepagurek reopened this Feb 22, 2023
@davepagurek davepagurek changed the title Uncaught TypeError: this._storeModeAndMaxes is not a function Document parameters of p5.Color constructor Feb 22, 2023
@davepagurek
Copy link
Contributor

Although it's possible to use new p5.Color, it has two required parameters (the p5 instance, and the initial color value), but these are not mentioned with @param in the doc comments here: https://github.com/processing/p5.js/blob/v1.5.0/src/color/p5.Color.js#L29

A fix for this issue would add documentation for these so that they show up in the p5 reference (and, even better, mention that color(...) is the easiest way to make a new color.)

@Pomax
Copy link
Author

Pomax commented Feb 22, 2023

Is it the "easiest" way, or is it the "recommended" way? Because if it's the latter, the docs should probably use some design language that makes it clear that this page is an implementation document and is only useful to folks who are mixing P5js with other code (e.g. using bare P5 inside of a larger JS app) rather than being part of the API for writing P5js code.

(it's mostly information for folks who are mix-and-matching regular JS with P5js interspersed)

@davepagurek
Copy link
Contributor

I think we can go with "recommended" as the language. In general the functions that create objects for you internally handle creating the objects on the right p5 instance, and using a constructor manually is for advanced users.

It's a bit of a limitation of our current docs system that we don't have a good way of using color() as the code snippet on this page https://p5js.org/reference/#/p5.Color. I'm not too familiar with how our docs system works, but it would be a useful enhancement for other classes too (e.g. p5.Vector/createVector() also would benefit from this)

@alex-key
Copy link

alex-key commented Feb 26, 2023

Hey, guys! I did not fine a better place to ask, so, please, let me know if there is an issues/discussion/forum which is a proper place.

Question is related to p5js documentation.
I started using p5js just recently and I think that documentation is the worst part of this great project. The tool itself is awesome, but the docs looks like a website from 2000-s and UX is pretty poor. I wondering is there any plan/roadmap on updating documentation and have a modern Design + better UX? I have some experience with tools like astro, vitepress and want to help with this. Thank you!

@Pomax
Copy link
Author

Pomax commented Feb 27, 2023

@alex-key definitely want to at least file a separate issue for that, since it's not related to p5.Color?

@alex-key
Copy link

@Pomax yeah, that's what I asked in the first statement.
Though this is the only one documentation issue which I found

@limzykenneth
Copy link
Member

@alex-key The documentation system is definitely not great and is something I want to work on for a long time now. The underlying system is a system called YUIDoc and the static page generation is using Assemble, both of which are abandoned by their original authors.

We have specific requirements that basically precludes us from switching to other stacks.

  • We want to keep inline documentation as that is an important aspect of p5.js' codebase
  • We want to be able to run interactive examples on individual pages
  • We want to have control over the exact layout and design of the individual pages
  • We want the ability to do internationalization easily (which is my main focus at the moment)

For inline documentation, YUIDoc is the only one currently (that I know of) that uses the JSDoc format and generate a JSON file that we can use for rendering (JSDoc last I checked does not generate a JSON file containing the documentation in a structured format). For static pages, I'm open to moving away from Assemble but we have the internationalization bits built alongside it so any new system will need to integrate our existing system without needing us to rebuild/reformat all our existing translations.

A new website is currently in discussion in the very early stages so that is also another reason to not go all in on a redesign/rebuild if the new website eventually goes with something completely different.

That's all a bit long and should really be another issue as mentioned 😅

@alex-key
Copy link

@limzykenneth Thank you, Kenneth for the detailed explanation. I will create another issue and copy your answer there.
@Pomax Sorry for polluting this one, but it helped a lot!

@sawaisinghh
Copy link
Contributor

Hey @davepagurek .
I want to work on this issue. Can you please describe this issue bit more?

@davepagurek
Copy link
Contributor

Thanks @sawaisinghh! The idea would be to edit the doc comments for the p5.Color constructor here: https://github.com/processing/p5.js/blob/v1.5.0/src/color/p5.Color.js#L29

We'd like it to look similar to what we've done for other classes, p5.Graphics, which describes each required parameter:

* @param {Number} w width
* @param {Number} h height
* @param {Constant} renderer the renderer to use, either P2D or WEBGL
* @param {p5} [pInst] pointer to p5 instance

We should also mention in the description of the class (in the area before the @class line) that color() is the recommended way to create an instance of this class (and maybe link to the docs for that function.) This way, people viewing the documentation will know that:

  • they should probably make an instance using color(...), not new p5.Color(...)
  • if they really do want to use the constructor directly, it's clear what they need to pass in as parameters in order for it to work

@sawaisinghh
Copy link
Contributor

sawaisinghh commented Mar 4, 2023

Thanks @davepagurek for explaning very well. I understood the issue. I can fix it. Can you please assign it to me.

@davepagurek
Copy link
Contributor

Thanks @sawaisinghh!

davepagurek added a commit that referenced this issue Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants