-
Notifications
You must be signed in to change notification settings - Fork 473
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
Color Comments #70
Color Comments #70
Conversation
I added my comments below the TODO's. The `color('white')` constructor I implemented is lowercase, similar to `rgb()`, `hsl()`, etc. but I can see why it could be argued to be uppercase. I changes all occurrences here to lowercase, but if I should change to implementation to uppercase, let me know.
OK, the convention for JavaScript constructor functions is Pascal case, as is the convention for JavaScript types, e.g., |
So in that case should we also make |
I meant to just open PRs into Cesium for things, not have the discussion in the .md file. Moving the comments here:
Make it uppercase throughout.
We should follow the CSS3 Colors spec. See what it says.
We would just pre-process them, but I agree I don't like them either and they don't fit into a strict grammar.
CSS doesn't have a
OK, just add case-insensitive before the first type our spec says "keyword"
Please do this soon so we can keep the spec and implementation in sync.
No, think of them as regular functions that return a |
Sorry, my confusion. |
OK |
The spec does not specifically mention empty values. |
OK, white is fine. Please update our spec in this branch. |
Updated. |
Nicely done! |
I added my comments below the TODO's.
The
color('white')
constructor I implemented is lowercase, similar torgb()
,hsl()
, etc. but I can see why it could be argued to be uppercase. I changed all occurrences here to lowercase, but if I should change to implementation to uppercase, let me know.