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

Proposed New Theme Fields #81

Open
VisenDev opened this issue Jul 30, 2024 · 4 comments
Open

Proposed New Theme Fields #81

VisenDev opened this issue Jul 30, 2024 · 4 comments

Comments

@VisenDev
Copy link
Collaborator

widget_borders: enum{only_necessary, always} = .only_necessary,

Option to enable borders on widgets that don't usually have borders (Buttons being the prime motivator)

default_corner_rounding: ?f32 = null,

Option to override the default rounding of corners. Themes with squared-off corners are now possible.

@david-vanderson
Copy link
Owner

I've been thinking about this. Right now each widget has defaults that can be overridden through the Options passed in. Maybe widgets need to first override their defaults according to these new theme fields, and then override those with the Options passed in.

Does that seem right to you?

@VisenDev
Copy link
Collaborator Author

VisenDev commented Aug 5, 2024

Does that seem right to you?

That makes sense to me. The logical priority seems to be: default widget settings < theme settings < callsite option settings

Building on that, I find Theme containing several Options a little confusing. Its also strange having a accent theme color setting that does one thing, and a style_accent Option field that does another. Same thing applies for err and style_err.

I may look into refactoring the Theme fields a little to make them more self documenting. I'm also thinking about removing style_err and style_accent and replacing them with something like

//formerly style_accent
dropdown_background: dvui.Color,
dropdown_text: dvui.Color,
dropdown_border: dvui.Color,

Alternatively, the non-Option fields of Theme could be replaced to also use Options as this would also make Theme more consistent (eg, style_normal: dvui.Option)

Its just not entirely clear why some types of ui elements have their own specific dvui.Color fields and others have Options

@VisenDev
Copy link
Collaborator Author

VisenDev commented Aug 5, 2024

Also it might make sense to change the implementation of Font so that size is not a direct property of a Font.

This would make it possible to change this

font_body: Font,
font_heading: Font,
font_caption: Font,
font_caption_heading: Font,
font_title: Font,
font_title_1: Font,
font_title_2: Font,
font_title_3: Font,
font_title_4: Font,

to something more like this

//Font IDS
font_body: Font,
font_heading: Font,
font_caption: Font,
font_title: Font,

//Font Sizes
font_body_size: f32,
font_heading_size: f32,
font_caption_size: f32,
font_caption_heading_size: f32,
font_title_size_1: f32,
font_title_size_2: f32,
font_title_size_3: f32,
font_title_size_4: f32,
font_title_size_5: f32,

The advantage of this would be less duplicated information when defining a theme - since often all font fields use the same font, eg,

    .font_body = .{ .size = size, .name = "Pixelify" },
    .font_heading = .{ .size = size, .name = "Pixelify" },
    .font_caption = .{ .size = size, .name = "Pixelify" },
    .font_caption_heading = .{ .size = size, .name = "Pixelify" },
    .font_title = .{ .size = size * 2, .name = "Pixelify" },
    .font_title_1 = .{ .size = size * 1.8, .name = "Pixelify" },
    .font_title_2 = .{ .size = size * 1.6, .name = "Pixelify" },
    .font_title_3 = .{ .size = size * 1.4, .name = "Pixelify" },
    .font_title_4 = .{ .size = size * 1.2, .name = "Pixelify" },

Another alternative to make the sizes definitions simpler would be defining a base font size and having all different display types just scale that base size up or down appropriately.

//Font Sizes
font_base_size: f32,

//default font scaling
font_body_scalar: f32 = 1,
font_heading_scalar: f32 = 1.5,
font_caption_scalar: f32 = 1.4,
font_caption_heading_scalar: f32 = 1.6,
font_title_scalar_1: f32 = 2,
font_title_scalar_2: f32 = 1.8,
font_title_scalar_3: f32 = 1.6,
font_title_scalar_4: f32 = 1.4,
font_title_scalar_5: f32 = 1.2,

This would simplify the Theme font declaration above to this

    .font_body = "Pixelify",
    .font_heading = "Pixelify",
    .font_caption = "Pixelify",
    .font_title = "Pixelify",
    .font_size = 15,

@david-vanderson
Copy link
Owner

I sympathize with the idea of making themes easier to create, but we have to balance that with the assumption that creating themes is a rare task - I very much appreciate your effort here! Let's address that later.

I'll experiment with adding a corner radius override in the themes and see how that goes.

I agree that the way Themes have Options inside is a little weird. Originally most of the stuff in Theme was duplicated 3 times (normal, accent, error). The intent is to make it easy for a dev using dvui to say "I want this control to look accented/dangerous" without tying it to a specific control.

style_accent is used for labels and icons in menus and for checked checkboxes and radio buttons. style_err is only used in the example button.

I'm very open to suggestions here - I was trying to prevent Theme from having a huge list of colors for every dvui widget. I'm already a bit bummed that it has 5 color_fill variations, but haven't thought of a better way yet.

Thanks for thinking through this stuff with me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants