Tatoeba / tatoeba2

Tatoeba is a platform whose purpose is to create a collaborative and open dataset of sentences and their translations.
https://tatoeba.org
GNU Affero General Public License v3.0
714 stars 132 forks source link

Fix user list sentence count overflow #3119

Open cblanken opened 7 months ago

cblanken commented 7 months ago

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 commented 7 months ago

Thanks for taking time to work on this, @cblanken :+1:

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 commented 7 months ago

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 commented 7 months ago

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

cblanken commented 7 months ago

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.