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

feat: display each font text with the corresponding font in theme settings #1023

Merged
merged 5 commits into from
Apr 26, 2024

Conversation

Dinis-Esteves
Copy link
Contributor

@Dinis-Esteves Dinis-Esteves commented Apr 21, 2024

I am submitting a pull request where I have made changes to the source code to enhance the font selection dropdown menu on the website.

Now, each font name in the dropdown is displayed in its corresponding font style.

This update makes the font selection process more intuitive and user-friendly, as users can see a preview of each font without having to test them one by one.

Old Appearance:

image

New Appearance:

image

Copy link
Member

@diogotcorreia diogotcorreia left a comment

Choose a reason for hiding this comment

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

Thank you for this PR, I like the final result a lot.

However, it results in users having to load all of these fonts, which is very wasteful if you're just using the default one.

The Google fonts API seems to have an option to only request a subset of characters, which would help with this: https://developers.google.com/fonts/docs/getting_started#optimizing_your_font_requests

I don't think there is a solution for the open-dyslexic font however, which is a problem, but I could investigate.

Let me know if you need help with this (either by here or just DM me on Discord/somewhere else).

src/components/ThemeSettings/ThemeSettings.js Outdated Show resolved Hide resolved
@diogotcorreia diogotcorreia added the enhancement New feature or request label Apr 21, 2024
@diogotcorreia
Copy link
Member

diogotcorreia commented Apr 26, 2024

I have made a few changes to your PR:

  • I've moved things from DropdownSelect and DropdownOption back to ThemeSettings, since the former two are generic and used for more things than just fonts;
  • You don't need to url encode the display names of fonts, it is possible to use encodeURIComponent, so I've used that instead;
  • I did my suggestion to add a loop with all available fonts instead of hardcoding them;
  • I made it so that the fonts required for preview are only loaded when you open the theme sidebar.

@diogotcorreia diogotcorreia changed the title [enhancement] Updated font dropdown to display each font text with the corresponding font feat: display each font text with the corresponding font in theme settings Apr 26, 2024
@diogotcorreia diogotcorreia merged commit 7539694 into leic-pt:master Apr 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants