-
Notifications
You must be signed in to change notification settings - Fork 41
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
Font theme rework #128
Font theme rework #128
Conversation
Can you speak to the motivation here? What does having a base_size make possible or easier? |
The main thing is that it makes it easier to adjust the base font size when adding new themes. Originally, Themes had to specify specific font sizes for each instance of a font. .font_body = .{ .size = 13, .name = "Vera" },
.font_heading = .{ .size = 13, .name = "VeraBd" },
.font_caption = .{ .size = 10, .name = "Vera" },
.font_caption_heading = .{ .size = 10, .name = "VeraBd" }, This made things difficult when trying to add new fonts for a theme, since if the default font size was too small, you would have to manually increment each This was unnecessarily difficult, so what I ended up doing on most of my themes was creating a const size = 18;
pub const jungle = Theme{
.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" },
//...
}; At this point, the Hence, its seems more straightforward to give each font a base_size parameter, and then scale that base_size up and down as necessary This makes implementing Thus, it seems like swapping out the font Hope that helps clarify. I'm not super tied to this change, I've just had this idea floating in my head for a while and wanted to test it out. Theres a couple things in I mentioned this here as well #81 |
An alternative approach would be to create a |
Thanks for the explanation! Will think about this, it seems like the base_size is a property of the theme maybe. I certainly have wanted a way to scale all the fonts in a theme (see fontSizeAdd() which I'm not happy with) , but I can't put my finger on the right way to do it. SerializableTheme is a good idea. Maybe that's the way to go. |
I'll look into sending an alternative PR that does this instead |
Very experimental changes to separate a Font's scale from a Font's base_size
I am planning on migrating all prebuilt themes to json to make them easier to edit and easier to create new themes. This is one of several changes I am going to experiment with to make Themes easier to work with.