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

Gradients in Color are poorly documented #663

Closed
altaic opened this Issue Jul 15, 2016 · 9 comments

Comments

Projects
None yet
5 participants
@altaic

altaic commented Jul 15, 2016

In trying to use Color.linear, it came to my attention that the Color docs don't explain its use, both of the example links are broken, and, more distressingly, the code itself doesn't shed any light:

{-| Abstract representation of a color gradient.
-}
type Gradient
  = Linear (Float,Float) (Float,Float) (List (Float,Color))
  | Radial (Float,Float) Float (Float,Float) Float (List (Float,Color))


{-| Create a linear gradient. Takes a start and end point and then a series of
“color stops” that indicate how to interpolate between the start and
end points. See [this example](http://elm-lang.org/examples/linear-gradient) for a
more visual explanation.
-}
linear : (Float, Float) -> (Float, Float) -> List (Float,Color) -> Gradient
linear =
  Linear


{-| Create a radial gradient. First takes a start point and inner radius.  Then
takes an end point and outer radius. It then takes a series of “color
stops” that indicate how to interpolate between the inner and outer
circles. See [this example](http://elm-lang.org/examples/radial-gradient) for a
more visual explanation.
-}
radial : (Float,Float) -> Float -> (Float,Float) -> Float -> List (Float,Color) -> Gradient
radial =
  Radial

This is one place where it might be nice to use type aliases or perhaps records. Regardless, either Gradient or the type of the functions that produce Gradient should be detailed.

@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Jul 15, 2016

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Jul 15, 2016

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@altaic

This comment has been minimized.

Show comment
Hide comment
@altaic

altaic Jul 15, 2016

To make matters worse, #434 exemplifies the problem with the current implementation and should be reopened.

No docs and no type safety do not make for happy developers.

altaic commented Jul 15, 2016

To make matters worse, #434 exemplifies the problem with the current implementation and should be reopened.

No docs and no type safety do not make for happy developers.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 8, 2016

Contributor

@altaic, the code that leads to a runtime error in #434 now lives in https://github.com/evancz/elm-graphics, so why don't you open an issue there?

Contributor

jvoigtlaender commented Aug 8, 2016

@altaic, the code that leads to a runtime error in #434 now lives in https://github.com/evancz/elm-graphics, so why don't you open an issue there?

@altaic

This comment has been minimized.

Show comment
Hide comment
@altaic

altaic Aug 9, 2016

Refer to src/Color.elm in this repo-- the code and terrible (lack of) documentation is there. Evan's knee-jerk closing of #434 and redirection to elm/graphics does not make it any less so.

altaic commented Aug 9, 2016

Refer to src/Color.elm in this repo-- the code and terrible (lack of) documentation is there. Evan's knee-jerk closing of #434 and redirection to elm/graphics does not make it any less so.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 9, 2016

Contributor

My comment was about the runtime error that you brought up in addition to the opening comment about documentation. That runtime error is not thrown from the pure Elm code (Color.elm) in core. The fact that more documentation inside core could help people running into this error less often is not something I disputed. Nor did I suggest you should open an issue at https://github.com/evancz/elm-graphics instead of this one here about the documentation.

Contributor

jvoigtlaender commented Aug 9, 2016

My comment was about the runtime error that you brought up in addition to the opening comment about documentation. That runtime error is not thrown from the pure Elm code (Color.elm) in core. The fact that more documentation inside core could help people running into this error less often is not something I disputed. Nor did I suggest you should open an issue at https://github.com/evancz/elm-graphics instead of this one here about the documentation.

@altaic

This comment has been minimized.

Show comment
Hide comment
@altaic

altaic Aug 9, 2016

I see; I don't use elm/graphics, so I didn't understand its relevance. Sorry for the confusion.

altaic commented Aug 9, 2016

I see; I don't use elm/graphics, so I didn't understand its relevance. Sorry for the confusion.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
Contributor

jvoigtlaender commented Aug 16, 2016

@jvoigtlaender jvoigtlaender added the docs label Aug 30, 2016

@drathier

This comment has been minimized.

Show comment
Hide comment

drathier commented Mar 5, 2018

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 22, 2018

Member

This code is moving out of core. It probably will be revived in elm-graphics and can be considered over there.

Member

evancz commented May 22, 2018

This code is moving out of core. It probably will be revived in elm-graphics and can be considered over there.

@evancz evancz closed this May 22, 2018

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