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

Fix user list sentence count overflow #3119

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

Conversation

cblanken
Copy link
Contributor

@cblanken cblanken commented Apr 11, 2024

This PR solves issue #3097

Hard-coding the base font size for the sentence list table to 16px is the simplest solution and appears to be the most common hard-coded font-size in the application. This keeps sentence list counts within the table boundary regardless of the user's browser configuration.

It would be nice to keep the font-size responsive but I think that would best be done in a larger redesign to address all the existing hard-coded font sizes.

Screenshots

Before Fix (with default browser font size set to 24px)

image

Before Fix (with default browser font size set to 20px)

image

With Fix

image

@jiru
Copy link
Member

jiru commented Apr 22, 2024

Thanks for taking time to work on this, @cblanken 👍

I saw in the issue page you mentioned that the problem comes from the browser default font size. Shouldn’t we set the default font size to 16px instead of setting it for the listSummary class specifically? My guess is that developers who worked on Tatoeba so far all used the default browser font size setting and built things with the assumption it would not change, so the same problem is likely to happen in other parts of the UI as well.

@cblanken
Copy link
Contributor Author

Shouldn’t we set the default font size to 16px instead of setting it for the listSummary class specifically? My guess is that developers who worked on Tatoeba so far all used the default browser font size setting and built things with the assumption it would not change, so the same problem is likely to happen in other parts of the UI as well.

You're right. There are probably some more font-size related issues, but I think that's more likely due to the static design of most of the app. I'll try setting the top-level default font size, but it's hard to say if some parts of the UI will be negatively affected without checking every page.

Here's a quick grep of all the hard-coded font-sizes from the *.css files.

webroot/50x.html
39:        font-size: 24px;
63:        font-size: 24px;

webroot/css/vocabulary/of.css
26:    font-size: 16px;

webroot/css/vocabulary/add_sentences.css
26:    font-size: 16px;

webroot/css/vocabulary/add.css
26:    font-size: 16px;
39:    font-size: 13px;

webroot/css/users/edit.css
25:    font-size: 18px;
48:    font-size: 18px;
69:    font-size: 18px;
80:    font-size: 20px;

webroot/css/reviews/of.css
26:    font-size: 24px;

webroot/css/pages/index.css
51:    font-size: 24px;

webroot/css/favorites/of_user.css
20:    font-size: 24px;

webroot/css/contributions/activity_timeline.css
80:    font-size: 16px;

webroot/css/stats/users_languages.css
52:    font-size: 12px;

webroot/css/sentences/show.css
113:    font-size: 16px;
122:    font-size: 16px;

webroot/css/layouts/elements.css
248:    font-size: 14px;
385:    font-size: 16px;
420:    font-size: 12px;
504:    font-size: 16px;
680:    font-size: 14px;
788:    font-size: 14px;
938:    font-size: 7px;
951:    font-size: 13px;
983:    font-size: 24px;
1054:    font-size: 16px;
1089:    font-size: 15px;
1107:    font-size: 16px;
1119:    font-size: 16px;
1125:    font-size: 16px;
1134:    font-size: 14px;
1248:    font-size: 13px;
1303:    font-size: 24px;
1393:    font-size: 24px;
1431:        font-size: 18px;
1539:    font-size: 16px;
1556:    font-size: 16px;
1560:    font-size: 20px;
1625:    font-size: 16px;
1717:    font-size: 14px;
1777:    font-size: 16px;

webroot/css/layouts/default.css
35:    font-size: 24px;  /* Preferred icon size */
159:    font-size: 13px;
219:    font-size: 24px;
258:    font-size: 16px;
304:    font-size: 14px;
376:    font-size: 15px;
390:    font-size: 20px;
406:    font-size: 14px;
447:    font-size: 15px;
496:    font-size: 24px;
517:    font-size: 12px;
557:    font-size: 14px;
618:    font-size: 15px;
665:    font-size: 15px;
777:    font-size: 13px;
798:    font-size: 20px;

And here's the dynamic font sizes

webroot/css/layouts/elements.css
87:    font-size: 1.2em;
94:    font-size: 0.8em;
532:    font-size: 1em;
1819:    font-size: 1.2em;

webroot/css/layouts/default.css
174:    font-size: 1.1em;
645:    font-size: 1.3em;
730:    font-size: 0.9em;

webroot/css/vocabulary/add_sentences.css
48:    font-size: 2em;

webroot/css/users/all.css
44:    font-size: 0.9em;
48:    font-size: 1.2em;

webroot/css/audio/import.css
20:    font-size: 1.1em;
26:    font-size: 1.1em;

webroot/css/tags/show_sentences_with_tag.css
46:    font-size: 0.8em;

webroot/css/sentences_lists/show.css
93:    font-size: 0.8em;
150:    font-size: 2em;

webroot/css/sentences/search.css
39:    font-size: 2em;

webroot/css/pages/index.css
34:    font-size: 0.9em;

webroot/css/pages/contact.css
24:    font-size: 1.4em;

src/Template/Layout/light.ctp
43:        .translations{font-size: 1.2em;}

@jiru
Copy link
Member

jiru commented Apr 22, 2024

Maybe @trang can give us some insight as she is more familiar with the existing CSS codebase.

@cblanken
Copy link
Contributor Author

I'll try setting the top-level default font size, but it's hard to say if some parts of the UI will be negatively affected without checking every page.

For what it's worth, I set the default font-size to 16px and clicked around the app for a while and didn't find anything out of the ordinary.

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