Orasund / elm-ui-widgets

Collection of reusable views for elm-ui.
https://orasund.github.io/elm-ui-widgets/
BSD 3-Clause "New" or "Revised" License
85 stars 11 forks source link

Material: add a 'current' color to the Palette #58

Closed cdevienne closed 3 years ago

cdevienne commented 3 years ago

In Material Design, we are supposed to be able to choose between the primary and secondary (and their variants) for the widgets.

I suggest to add a "current" color selector in the Palette that would be used by the widgets instead of always using the primary color.

Something like:

type ColorVariant
    = Primary
    | Secondary
--    | PrimaryVariant
--    | SecondaryVariant

type alias Palette =
    { current : ColorVariant
    }

selectColor : ColorVariant -> Palette -> Palette
selectColor variant palette =
    { palette | current = variant }
cdevienne commented 3 years ago

If I provide a PR for this and we agree on the API, would you consider doing a 4.0.0 soon?

Orasund commented 3 years ago

I though a bit about this issue... And I believe that

  1. This should be possible without a major change
  2. I do not want to have something like .current in my palette (because the palette should be stateless and .current is a state)

Here is the idea I currently have:

swapColor : Palette -> Palette
swapColor palette =
    { palette 
    | primary = palette.secondary
    , secondary = palette.primary
    -- and also swap the variants
    }

So now you could do something like

-- Inside your Main.elm

type ColorVariant
    = Primary
    | Secondary

selectColor : ColorVariant -> Palette -> Palette
selectColor variant =
    if Primary then identity else swapColor

Would that fit your needs?

Orasund commented 3 years ago

Also, do you have some reference where it says that one can decide between primary and secondary?

cdevienne commented 3 years ago

from https://material.io/design/color/the-color-system.html#color-theme-creation:

A primary color is the color displayed most frequently across your app's screens and components.

and

A secondary color provides more ways to accent and distinguish your product. Having a secondary color is optional, and should be applied sparingly to accent select parts of your UI.

cdevienne commented 3 years ago

I though a bit about this issue... And I believe that

1. This should be possible without a major change

2. I do not want to have something like `.current` in my palette (because the palette should be stateless and `.current` is a state)

[...]

Would that fit your needs?

Yes it would, thanks for your insight!

cdevienne commented 3 years ago

Ideally we would also have the alternative colors somehow stored inside the palette too, but it's easy to workaround too.

cdevienne commented 3 years ago

[...] Would that fit your needs?

Yes it would, thanks for your insight!

With a minor drawback (that I can live with): we never know if the current color is the real primary or not.

Orasund commented 3 years ago

Well, I believe a widget should not care if it is currently using the primary or secondary color. So for me that's more a feature than a drawback - but that's up to debate.

cdevienne commented 3 years ago

The widget no, indeed, but a high-level view or layout might. And having a secondary color in a 'primary' attribute may be confusing.

Orasund commented 3 years ago

The widget no, indeed, but a high-level view or layout might. And having a secondary color in a 'primary' attribute may be confusing.

Let's come back to this, once this really is an issue. For 3.0.0 this issue can be addresses using a swapColor function. Note, that 4.0.0 will the FINAL major change (for the unforeseeable future), so I'd like to do as much as possible in 3.X.X.

I'll be adding the function today and then close this issue (for now).

Orasund commented 3 years ago

Function is called Material.swapColor