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

Assign shield icons based on ref length #1086

Merged
merged 3 commits into from
Jun 1, 2024

Conversation

claysmalley
Copy link
Member

Fixes #86

This PR changes the behavior of shield icon selection. Instead of measuring the aspect ratio of the displayed text, it simply chooses by matching string length to the length specified at the end of the icon's filename (e.g. shield_us_i_2, shield_us_i_3).

Certain narrow characters (currently 1, I, i, l and ) are considered to be two-thirds of a width of all other characters. This ensures that 111, for example, is assigned a 2-digit variant icon if available.

Background

When I was working on #1083, it looked great on my computer screen (Firefox/Linux). Once it deployed, I checked it out on my phone (Firefox/Android), and to my disappointment, every Utah shield displayed the 3-digit variant icon. Alas, even #965 didn't fix the inconsistent text metrics across different platforms. Americana needs to choose the icon based on platform-independent factors.

image

@claysmalley claysmalley added bug Something isn't working shield-generator Issues specific to the shield library labels Jun 1, 2024
Copy link
Member

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sad but true. I’m approving this as a workaround at least. But long-term, I don’t think this will save us from having to choose a fixed font size for each design. Some two-digit shields currently size some single digits differently than other single digits based on their optical width. Choosing a font size will free us from having to fine-tune paddings, which has got to be the most frustrating aspect of designing new shields for this codebase.

We could use some rendering tests that exercise this change, particularly the exceptions.

shieldlib/src/shield.ts Outdated Show resolved Hide resolved
shieldlib/src/shield.ts Show resolved Hide resolved
@claysmalley claysmalley merged commit 7b75a99 into main Jun 1, 2024
4 checks passed
@claysmalley claysmalley deleted the clay-shields-assign-icon-by-length branch June 1, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working shield-generator Issues specific to the shield library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wide shields showing up for 1 & 2 digit shields at DPR 1
2 participants