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

Remove css nowrap from body #2015

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

giovanniTramonto
Copy link
Contributor

This sets document white-space back to normal (collapsed).
As mentioned in #1932 (comment) I would recommend this. IMO it’s better for responsive behavior and avoids many overrides in the tree, since in most cases nowrap is an exception.

@justvanrossum
Copy link
Collaborator

While I agree with the general idea, I am afraid of lots of subtle consmetic breakages, in list items, axis names, source names, etc. This affects a very broad range of elements.

@giovanniTramonto
Copy link
Contributor Author

giovanniTramonto commented Feb 11, 2025

Understandable. At the same time I was surprised how little the impact was. The only critical part I found was inside dialog layouts, like here https://github.com/googlefonts/fontra/blob/main/src/fontra/views/editor/editor.js#L2413
Those I did not change

@giovanniTramonto
Copy link
Contributor Author

I started to do an extensive UI test. Will add results soon

@giovanniTramonto
Copy link
Contributor Author

I tested varied scenarios with long placeholder text as well as interactions.
Screens are named by section and branch (--feature or --main).
In total I found 4 differences: 3 bugs (marked red) and 1 potential improvement IMO (marked orange)

Bildschirmfoto 2025-02-13 um 12 04 08

Screens are here https://drive.google.com/drive/folders/1yEHHulq3416DBPKfgLknFLFV5WVvcOB1?usp=sharing
(files affected by the difference are stared)

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