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

Recent change to dynamic scaling broke themes that use custom fonts #337

Closed
mustiikhalil opened this issue Jul 15, 2024 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@mustiikhalil
Copy link

mustiikhalil commented Jul 15, 2024

Hello,

Thanks for maintaining this library. We are facing an issue with the dynamic text scaling and custom fonts whenever we try to use the library after the latest update 2.3.1. In the latest release there is this following PR that adjusts the use of "setting the dynamic text twice". As seen below in the images the when using the markdownTheme in the green box it doesnt change the size of the text while the user is changing the text size.

The project:

We use the following SwiftUI view as an example:

struct ContentView: View {
    let text = """
    ### Hello, world!

    This is text
    """
    var body: some View {
        VStack(spacing: 10) {
            Text(text) // This should only display the text without any changes for the sake of sanity checks
                .font(.custom("menlo", size: size))
                .background(Color.red)

            Markdown(MarkdownContent(text)) // This renders the markdown properly
                .markdownTextStyle {
                    FontFamily(.custom("menlo"))
                    FontSize(size)
                }
                .background(Color.blue)

            Markdown(MarkdownContent(text))  // This fails to render the markdown properly
                .markdownTheme(.markdownTheme)
                .background(Color.green)
        }
        .padding()
    }
}

Theme:

extension Theme {
    static let markdownTheme = Theme()
        .heading3 { config in
            config.label
                .markdownTextStyle {
                    FontFamily(.custom("menlo"))
                    FontSize(28)
                }
        }
        .paragraph { config in
            config.label
                .markdownTextStyle {
                    FontFamily(.custom("menlo"))
                    FontSize(size)
                }
        }
        .text {
            FontFamily(.custom("menlo"))
            FontSize(size)
        }
}

Screenshots:
Default Sized text:

simulator_screenshot_880F589A-6DDA-42F6-A5BE-5E4B2154C9CB

When user sets text bigger:

simulator_screenshot_95C3E170-1248-4766-BECC-680DC86EF05C

Screenshots from 2.3.0:

Default Sized text:

simulator_screenshot_0AC0F883-3134-42A4-A015-04A64CD243D6

When user sets text bigger:

simulator_screenshot_EA1770B0-69E9-4CEC-8304-81393A389889

@mustiikhalil
Copy link
Author

cc: @gonzalezreal

@gonzalezreal
Copy link
Owner

Sorry, I am currently unavailable. I can look into this in a couple of weeks.

@gemigkaka
Copy link

@gonzalezreal Hi! Have you got any time to look into this yet? I could open a PR to fix it, but since that would just be a revert of this PR, I'm not sure how to proceed.

@gonzalezreal
Copy link
Owner

gonzalezreal commented Sep 4, 2024

Hi @gemigkaka,
I still haven't got the time to look into this. I need to review #314 again before reverting that PR. I will take a look as soon as I can.

@gonzalezreal gonzalezreal added the bug Something isn't working label Sep 4, 2024
@gonzalezreal
Copy link
Owner

Hi @mustiikhalil & @gemigkaka,

Sorry for taking so long to look into this. Thanks for your patience :)

The code introduced in #314 is correct, as previously the dynamic type scale was applied twice. The issue with the code you have provided in the description is that you are using an absolute point size for the font in the paragraph and heading3 styles, and this is causing the font scale to reset to 1.

public func _collectAttributes(in attributes: inout AttributeContainer) {
switch self.size {
case .points(let value):
attributes.fontProperties?.size = value
attributes.fontProperties?.scale = 1
case .relative(let relativeSize):
switch relativeSize.unit {
case .em:
attributes.fontProperties?.scale *= relativeSize.value
case .rem:
attributes.fontProperties?.scale = relativeSize.value
}
}
}

Before #314 this was working for your use case because the scale was being incorrectly applied twice.

If you remove the custom paragraph style from the custom theme in the description, you will see that the paragraph text scales properly while the heading text doesn't, because the FontSize(28) is resetting the scale to 1.

The recommended way of adjusting the font size for the different block and text styles is to use a relative size, either based on the parent font size (.em) or the root font size (.rem). Use an absolute point size only for the base text style.

The following theme scales the text properly when changing the dynamic type:

extension Theme {
  static let myTheme = Theme()
  .heading3 { config in
    config.label
    .markdownTextStyle {
      FontFamily(.custom("trebuchet MS"))
      FontWeight(.semibold)
      FontSize(.em(1.5))
    }
  }
  .text {
    FontFamily(.custom("menlo"))
    FontSize(16)
  }
}

I am closing this issue as the library is behaving as expected.

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