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

Fix bug when requesting italic gfonts #5623

Merged
merged 3 commits into from Oct 29, 2023
Merged

Conversation

dewet22
Copy link
Contributor

@dewet22 dewet22 commented Oct 29, 2023

What does this implement/fix?

This fixes the missing left-hand side of the font spec. Currently an italic font will be requested as <family>:wght@1,700 which fails with HTTP 400 "Invalid selector: Too many values in tuple" referring to the right-hand side. The correct selector would be <family>:ital,wght@1,700.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): n/a

Pull request in esphome-docs with documentation (if applicable): n/a

Test Environment

n/a, not hardware-specific.

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

 font:
   - file:
       type: "gfonts"
       family: "Archivo+Narrow"
       weight: 500
     id: font_weight
     size: 30
   - file:
       type: "gfonts"
       family: "Archivo+Narrow"
       weight: 700
       italic: true
     id: font_weight_italic
     size: 30
   - file:
       type: "gfonts"
       family: "Archivo+Narrow"
       italic: true
     id: font_italic
     size: 30
   - file:
       type: "gfonts"
       family: "Archivo+Narrow"
     id: font_default
     size: 30

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

Without this the font spec is incorrectly generated as e.g. `:wght@1,700` which leads to an HTTP 400 error of "Invalid selector: Too many values in tuple". This patch adds the required leading `ital,` axis to the selector in this edge case. I've tested it with all permutations of weight and italics specified/not specified and it seems to reliably download fonts now.
@jesserockz jesserockz added this to the 2023.10.4 milestone Oct 29, 2023
@jesserockz jesserockz merged commit f36ec7c into esphome:dev Oct 29, 2023
44 checks passed
@dewet22 dewet22 deleted the patch-1 branch October 29, 2023 19:09
@jesserockz jesserockz mentioned this pull request Oct 30, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants