Button has no border

[brianmay]

Checklist

• [x] Check other issues and make sure that it is not reported yet.

Versions and Environment

Elixir:

# elixir -v
Erlang/OTP 22 [erts-10.7.2.3] [source] [64-bit] [smp:20:20] [ds:20:20:10] [async-threads:1] [hipe]

Elixir 1.10.4 (compiled with Erlang/OTP 22)

Erlang:

# erl -v
Erlang/OTP 22 [erts-10.7.2.3] [source] [64-bit] [smp:20:20] [ds:20:20:10] [async-threads:1] [hipe]

Eshell V10.7.2.3  (abort with ^G)

Scenic:

# mix deps | grep scenic
* scenic (Hex package) (mix)
  locked at 0.10.2 (scenic) 6f1445a7
* scenic_driver_nerves_rpi (Hex package) (mix)
  locked at 0.10.1 (scenic_driver_nerves_rpi) 4a3995b3
* scenic_driver_nerves_touch (Hex package) (mix)
  locked at 0.10.0 (scenic_driver_nerves_touch) 80e69499

OS:

Debian/bulleye and RIP3 Nerves image.

Steps to reproduce

        theme =
              :info
              |> Scenic.Primitive.Style.Theme.preset()
              |> Map.put(:background, :black)
              |> Map.put(:border, :white)

            graph =
              add_button(graph, button.name, {:state_hard_off, button.id}, x, y,
                theme: theme,
              )

Expected Behavior

Button should appear with border.

Actual Behavior

Button has no border.

Additional Comments

From https://github.com/boydm/scenic/blob/890d26431618519551665d3e2b7eb7800b81a5de/lib/scenic/component/button.ex#L218 it looks like a border will only be drawn if theme == :light or :dark.

defp do_special_theme_outline(:dark, graph, border) do
    Graph.modify(graph, :btn, &update_opts(&1, stroke: {1, border}))
  end

  defp do_special_theme_outline(:light, graph, border) do
    Graph.modify(graph, :btn, &update_opts(&1, stroke: {1, border}))
  end

  defp do_special_theme_outline(_, graph, _border) do
    graph
  end

But the documentation says nothing about this requirement: https://hexdocs.pm/scenic/Scenic.Component.Button.html#module-theme

[brianmay]

If there is some way of providing a custom value of :stroke to the :btn component that I completely missed, then this should be added to the documentation for the Button component.

[boydm]

Interesting. I'm doing quite a bit of work in Scenic right now, and may not get to this. If you want to dig around in the button code, feel free, but I won't be able to get to it for a bit.

[boydm]

One more thought. I'm not convinced the themes work as well overall as I'd like. I'm open to suggestions on how to improve them.

[brianmay]

Boyd Multerer notifications@github.com writes:

One more thought. I'm not convinced the themes work as well overall as I'd like. I'm open to suggestions on how to improve them.

What issues do you have with the themes (apart from this one) that you would like to see improved? -- Brian May brian@linuxpenguins.xyz https://linuxpenguins.xyz/brian/

[boydm]

The main issue I'm wrestling with right now is now themes inherit down a graph.

The original goal was to set a theme at the top of your graph and then it gets pass down to any components as an automatic sort of style. it is weird that this is the only style that behaves this way. Also proving to be very difficult, to do in a world where there can be multiple graphs per scene. You could actually have multiple graphs before, but theme inheritance was broken there too.

Thinking of making themes a thing you have to set on every component. There are pros and cons to this. It clearly means more typing to assign the themes. (maybe that's not so bad?). But it puts theme assignment on the thing actually consuming it, which is more clear.

@crertel I would like your opinion...

[Eiji7]

@boydm I did not look at source code of themes, but if I would do it then it would be something like:

defmodule Scenic.Components.Input do
  # would create Scenic.Components.Input.Theme struct with [:custom_key] fields
  use Scenic.Theme, [:custom_key]

  def apply_theme(graph, theme, button_theme \\ []) do
    # …
  end

  # …
end

defmodule Scenic.Theme do
  defstruct [:background]

  # here theme may be a struct with generic data like background, text color, padding etc.
  # and button_theme would handle all custom theming provided by Button component
  # for example: [custom_key: "custom value"]
  @callback apply_theme(graph, theme :: t(), button_theme :: struct()) :: graph when graph: Scenic.graph()

  # …
end

defmodule MyApp.Scene do
  use Scenic.Scene

  # …

  def handle_call(:something_happen, _from, graph: graph) do
    graph = Scenic.Graph.apply_theme(graph, background: {:color, :yellow})
    # …
  end
end

defmodule MyApp.Theme do
  alias Scenic.Components.{DateInput, Input}

  mode :dark do
    use MyLib.Theme, mode: :dark
    theme Input, [background: {:color, :red}], custom_key: "custom value"
    # set theme for CustomButton as same as in Button except background
    theme DateInput, [Input, background: {:color, :blue}], date_custom_key: "custom value"
  end

  mode :light do
    use MyLib.Theme, mode: :bright
    # …
  end
end

config :scenic, theme: {MyApp.Theme, :dark}
# or maybe theme configuration for each viewport?

With such API we can:

1. call a library to provide a default theming
2. apply globally some themes
3. use previously defined component themes to style other component themes
4. simply support a way to apply custom theme by hand
5. support multiple modes and allow easily switching it

What do you think about it?

[vacarsu]

I think the problem with the macro approach is you start making assumptions about how the developer wants to theme their application. I say keep the theming api relatively low level and let the developers build what they need around it. I think the main power of scenic is it doesn't make opinions how you should do things (for the most part). While that can be a double edged sword it allows developers to have more flexibility and creativity in developing a system around scenic. Of course I could be completely wrong and missing the boat here. :)

[boydm]

I'm going to close this as by design.

You can always make your own version of a button, but I'd like Scenic UI to have a somewhat common look and feel.