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

Make API support table circles a more consistent size #1802

Merged
merged 2 commits into from
Mar 5, 2023

Conversation

mhsmith
Copy link
Member

@mhsmith mhsmith commented Mar 4, 2023

The fonts in the Furo style sheet are:

-apple-system,BlinkMacSystemFont,Segoe UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji;

On macOS, this resolves to the system font SF, where the black and white circles are the same size, and fairly large. But on Windows it uses Segoe UI, where the black circle is much smaller than the white one.

This PR makes the tables use fonts whose circles are equally-sized, and fit in well with the specified font size.

I considered using other Unicode circles instead, but none of them worked well enough:

  • The "medium" circles appeared as emojis by default, which stopped them responding to dark mode. There's a variation character which is supposed to switch them back to text mode, but it only worked on Windows, not macOS.
  • The "large" circles were not supported by SF, Arial or Lucida Grande, causing the browser to fall back on other fonts with inconsistent alignment.

Test document for future reference:

<!DOCTYPE html>
<html>
<head><title>Circle test</title></head>
<body>
<p>Plain: &#x25cb; &#x25cf;</p>
<p>Medium: &#x26aa; &#x26ab;</p>
<p>Medium text: &#x26aa;&#xfe0e; &#x26ab;&#xfe0e;</p>
<p>Large: &#x25ef; &#x2b24;</p>
</body>
</html>

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@mhsmith
Copy link
Member Author

mhsmith commented Mar 4, 2023

Also fixed the admonition in pack.rst which was added in #1786, but was being omitted by the readthedocs build because admonition requires a title.

And updated the readthedocs configuration to make warnings fatal, so problems like this will not be silent in the future.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks good to me - and the alt-text/role handling is a nice touch, too.

@freakboy3742 freakboy3742 merged commit 26ed94e into beeware:main Mar 5, 2023
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

Successfully merging this pull request may close these issues.

2 participants