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

Add toggle-fullscreen control with temp icons. #761

Merged
merged 5 commits into from
Aug 31, 2023
Merged

Conversation

fabiocaccamo
Copy link
Collaborator

@fabiocaccamo fabiocaccamo commented Aug 30, 2023

Fix #189

@fabiocaccamo fabiocaccamo self-assigned this Aug 30, 2023
@justvanrossum
Copy link
Collaborator

For icons, I think I prefer the "minimize" and "maximize" icons from https://tabler-icons.io/

image

We can use these as-is (filename and all), and put them in the /tabler-icons folder. Perhaps we need to tweak the stroke thickness and size to match the other tool icons (with CSS).

@fabiocaccamo fabiocaccamo marked this pull request as ready for review August 31, 2023 10:00
@justvanrossum
Copy link
Collaborator

Can you try adding CSS to tweak the icon stroke width for these? They are a bit heavy compared to the one directly to the left.

@fabiocaccamo
Copy link
Collaborator Author

fabiocaccamo commented Aug 31, 2023

I think that the icon that has a different stroke width is the "bullseye", actually the stroke width of the fullscreen icons is the same of plus and minus icons.

Changing the order of the icons should not have any impact on the visual harmony.

@justvanrossum
Copy link
Collaborator

It's still too heavy, even if I compare to plus and minus.

src/fontra/views/editor/editor.js Outdated Show resolved Hide resolved
src/fontra/views/editor/editor.js Outdated Show resolved Hide resolved
@justvanrossum justvanrossum merged commit 5be4254 into main Aug 31, 2023
3 checks passed
@justvanrossum justvanrossum deleted the fullscreen branch August 31, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add full screen mode
2 participants