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

Change type of LineStyle.dashing from "List Int" to "List Float" #562

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@pisys
Contributor

pisys commented Apr 18, 2016

Why?

  • Makes relative definition (values between 0 and 1) of LineStyle properties possible
  • LineStyle.width also is Float, so it'd be consistent

The background I'm making this PR on: I use to define shapes with relative coordinates and dimensions and finally scale the whole group of shapes to the actual size of the canvas. With LineStyle.dashing being of type List Int, this is not possible.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Apr 18, 2016

Contributor

Why would you not also change the type of LineStyle.dashOffset, then? (ignoring for the moment that it is currently defunct, see https://github.com/elm-lang/core/pull/535)

Certainly if dashing is a List Float, then dashOffset should be a Float.

Contributor

jvoigtlaender commented Apr 18, 2016

Why would you not also change the type of LineStyle.dashOffset, then? (ignoring for the moment that it is currently defunct, see https://github.com/elm-lang/core/pull/535)

Certainly if dashing is a List Float, then dashOffset should be a Float.

@pisys

This comment has been minimized.

Show comment
Hide comment
@pisys

pisys Apr 18, 2016

Contributor

Yes, I forgot to mention that that change should account for dashOffset too, of course.

Contributor

pisys commented Apr 18, 2016

Yes, I forgot to mention that that change should account for dashOffset too, of course.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 10, 2016

Member

Sorry for the slow reply. It has been busy times. All the Graphics.* modules have moved to evancz/elm-graphics so it makes sense to retarget stuff like this.

Member

evancz commented May 10, 2016

Sorry for the slow reply. It has been busy times. All the Graphics.* modules have moved to evancz/elm-graphics so it makes sense to retarget stuff like this.

@evancz evancz closed this May 10, 2016

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