Skip to content
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

Text only uses theme inside components #162

Open
lawik opened this issue May 21, 2019 · 3 comments
Open

Text only uses theme inside components #162

lawik opened this issue May 21, 2019 · 3 comments
Labels
bug Something isn't working

Comments

@lawik
Copy link

lawik commented May 21, 2019

Checklist

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

Versions and Environment

Elixir: 1.8.1-otp-21
Erlang: 21.2.2
Scenic: 0.10.0
OS: MacOS

Steps to reproduce

Graph.build(font_size: 22, font: :roboto_mono, theme: :light)
  |> text("System")
  |> button("System")

Expected Behavior

Text would default to a dark color for the light theme and be visible.

Actual Behavior

Text is not a component, it is a primitive, which means it ignores the theme and renders white by default. The text ends up invisible.
The text inside the button is handled as expected and visible.

Additional Comments

After some discussion with Emilio I believe this works as intended but I'd say it doesn't work the way you'd expect. I think following the theme is the work of the component, not the primitive. But the default way of putting arbitrary text on the screen is using the primitive. So we get no good default for the theming. Would it be reasonable for text to pick up the theme for default text color?

I think there is a potential for UI developer incredulity at why text just doesn't respect the theme setting. Do we want a component that simply wraps text and does the theming? What is that called? A Label? A ThemedText?

@boydm
Copy link
Collaborator

boydm commented May 21, 2019

I see the point you are making. Good question on if it should be a change to the text primitive to pick up the default from them or a label control.

I'm leaning toward updating the primitive itself.

@lawik
Copy link
Author

lawik commented May 21, 2019

I took a brief look at it hoping I could throw a PR in here for that case. But I didn't quite follow where that piece of action would have to happen from my brief attempt.

If you could point me in the right direction I'd be happy to make a proof-of-concept we could consider.

@boydm
Copy link
Collaborator

boydm commented Jul 2, 2019

This isn't so straight-forward. Text is just a primitive and thus doesn't have a natural place to add code telling to pick up the contents of a theme. Instead it just inherits whatever the current font styles are form above it in the graph tree.

Probably the best place is if a theme is set during the Graph.build(...) function, then extract and set the font styles. Assuming they are not already set explicitly by the dev.

Memory is fuzzy. May have already done this? Need to take a look.

@crertel crertel added the bug Something isn't working label Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants