Make WordPress Core

Opened 4 months ago

Closed 8 weeks ago

#60851 closed defect (bug) (fixed)

Bulk edit and quickedit needs more space for longer translations

Reported by: zodiac1978's profile zodiac1978 Owned by: swissspidy's profile swissspidy
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.3
Component: Quick/Bulk Edit Keywords: has-screenshots has-patch
Focuses: ui Cc:

Description

In #25753 we introduced more space (5em was the default, we needed 7em) for .locale-de-de .inline-edit-row fieldset label span.title in the German locale.

After that, #28303 added this change to the new formal locale .locale-de-de-formal

But in #33212 this was removed again by @ocean90 and the new default was 6em.

This wasn't enough for ru_RU which got 8em.

In WordPress 6.2 the string "Parent" was translated with "Eltern" and for this 6em was okay:
https://translate.wordpress.org/projects/wp/6.2.x/admin/de/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=16083988&filters%5Btranslation_id%5D=105887217

But in WordPress 6.3 this was changed to "Übergeordnet":
https://translate.wordpress.org/projects/wp/6.3.x/de/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=16507337&filters%5Btranslation_id%5D=109126351

Now the translation is too long and 6em space is not sufficient anymore.

Re-adding the 7em width would solve this issue again.

There is also the same problem with the Polish locale which has a very long translation:
https://translate.wordpress.org/projects/wp/dev/pl/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=16087645&filters%5Btranslation_id%5D=106835593

Attachments (15)

Bildschirmfoto 2024-03-27 um 11.51.51.png (19.8 KB) - added by zodiac1978 4 months ago.
Broken layout in German locale (bulk edit)
Bildschirmfoto 2024-03-22 um 11.03.37.png (3.3 KB) - added by zodiac1978 4 months ago.
Broken layout in German locale (quick edit)
Bildschirmfoto 2024-03-27 um 14.16.17.png (21.5 KB) - added by zodiac1978 4 months ago.
Broken layout in Polish locale
Bildschirmfoto 2024-03-27 um 11.52.10.png (19.9 KB) - added by zodiac1978 4 months ago.
Fixed layout in German locale (bulk edit) - 7em space
Bildschirmfoto 2024-03-22 um 11.03.49.png (3.3 KB) - added by zodiac1978 4 months ago.
Fixed layout in German locale (quick edit) - 7em space
60851.diff (750 bytes) - added by zodiac1978 4 months ago.
Modify width to 7em only for de (formal) locale
2024-05-29_21-13-42.png (95.2 KB) - added by oglekler 2 months ago.
2024-05-29_21-14-04.png (95.3 KB) - added by oglekler 2 months ago.
Bildschirmfoto 2024-05-30 um 16.56.14.png (43.7 KB) - added by zodiac1978 2 months ago.
Broken layout in Quickedit view
Bildschirmfoto 2024-05-31 um 09.23.11.png (19.3 KB) - added by zodiac1978 2 months ago.
Word break layout issue with CSS version 1
Bildschirmfoto 2024-05-31 um 09.24.01.png (18.1 KB) - added by zodiac1978 2 months ago.
Word break layout issue with CSS version 1 and more issues (Quickedit)
60851.2.diff (621 bytes) - added by zodiac1978 2 months ago.
Patch #2 targeting every de locale
60851.3.diff (849 bytes) - added by zodiac1978 2 months ago.
Fixing incorrect patch after some wrong copy&paste
60851.4.diff (1.2 KB) - added by zodiac1978 2 months ago.
Now mobile fix also included (like for ru_RU and lt_LT)
60851.5.diff (1.6 KB) - added by zodiac1978 2 months ago.
Additionally fix some minor layout issue for the quickedit status label

Download all attachments as: .zip

Change History (37)

@zodiac1978
4 months ago

Broken layout in German locale (bulk edit)

@zodiac1978
4 months ago

Broken layout in German locale (quick edit)

@zodiac1978
4 months ago

Broken layout in Polish locale

@zodiac1978
4 months ago

Fixed layout in German locale (bulk edit) - 7em space

@zodiac1978
4 months ago

Fixed layout in German locale (quick edit) - 7em space

@zodiac1978
4 months ago

Modify width to 7em only for de (formal) locale

#1 @zodiac1978
4 months ago

  • Keywords has-patch dev-feedback added

I think there are 3 possible solutions here:

  • We could change the translation to a shorter string - I think this was an intentional change, because "Übergeordnet" is much better than the literal translation of "Parent" with "Eltern".
  • We could switch to a better CSS design using flexbox or grid, but as I understand, this would need more markup changes and as the admin design will get a refresh in the future this sounds like the wrong approach.
  • We could re-introduce the bigger width for the German locale (which is provided in the patch).

This does not fix the problem with the Polish translation, but I pinged someone from the Polish community to take a look. Maybe they can fix it with shortening the string.

Edit: Here is the link for the Polish translation.

Last edited 4 months ago by zodiac1978 (previous) (diff)

#2 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 6.6

#3 follow-up: @oglekler
3 months ago

I think there can be simple enough CSS patch: flex to label, margin:auto to children, line-height:1.4 for .title. I believe that making exceptions for different locales is not the right things to do.

#4 in reply to: ↑ 3 ; follow-up: @zodiac1978
3 months ago

Replying to oglekler:

I think there can be simple enough CSS patch: flex to label, margin:auto to children, line-height:1.4 for .title. I believe that making exceptions for different locales is not the right things to do.

I do not think there is an easy CSS patch possible with the current markup. We either make an exception for some locales or we raise the width for all. It is an easy fix which was already used in the past.

Fiddling with markup and CSS will mean to punt this ticket as this will be not ready for 6.6 in time.

This ticket was mentioned in Slack in #core by nhrrob. View the logs.


2 months ago

#6 follow-up: @nhrrob
2 months ago

  • Milestone changed from 6.6 to 6.7

We have reviewed this in today's bug scrub.
Looks like it won't be ready for 6.6

Punting to the next release 6.7.

#7 in reply to: ↑ 6 @zodiac1978
2 months ago

  • Keywords 2nd-opinion added

Replying to nhrrob:

We have reviewed this in today's bug scrub.
Looks like it won't be ready for 6.6

Punting to the next release 6.7.

This is such a disappointing decision. There is a patch available, and the other options do not make any sense with the upcoming redesign of the admin area in mind. Why don't we fix it in the meantime?

This locale specific solution was used before and for other problems of the same kind.

Pinging @ocean90 and @swissspidy for a second opinion on this matter.

#8 in reply to: ↑ 4 ; follow-up: @swissspidy
2 months ago

I think there can be simple enough CSS patch: flex to label, margin:auto to children, line-height:1.4 for .title. I believe that making exceptions for different locales is not the right things to do.

I do not think there is an easy CSS patch possible with the current markup. We either make an exception for some locales or we raise the width for all. It is an easy fix which was already used in the past.

Fiddling with markup and CSS will mean to punt this ticket as this will be not ready for 6.6 in time.

Well, has this been tested already? Maybe as a quick mockup in the browser's dev tools?

If it's too complex then something like the current patch is an OK solution for now, unless many more locales are affected. Keep in mind though it only works for de_DE, not de_AT or de_CH.

There is also the same problem with the Polish locale which has a very long translation

Are any other locales affected?

#9 in reply to: ↑ 8 @zodiac1978
2 months ago

Replying to swissspidy:

Well, has this been tested already? Maybe as a quick mockup in the browser's dev tools?

I couldn't get this working with just CSS in the browser dev tools. This is because every row has no connection to the other rows, therefore the width of one column has no effect to other columns.

I think the checkbox in the bulk edit variant make it even more difficult.

If it's too complex then something like the current patch is an OK solution for now, unless many more locales are affected. Keep in mind though it only works for de_DE, not de_AT or de_CH.

As de_AT and de_CH are copying from DE, every of those locales is affected:
de_DE, de_DE-formal, de_AT, de_CH, de_CH-informal

We could also try to use an attribute selector to shorten this up:
Something like body[class*="locale-de-"]

Are any other locales affected?

I am not 100% sure, but checked many and the DE variants and Polish were the only ones I found.

This ticket was mentioned in PR #6675 on WordPress/wordpress-develop by @oglekler.


2 months ago
#10

This ticket was mentioned in PR #6676 on WordPress/wordpress-develop by @oglekler.


2 months ago
#11

Line height change

Trac ticket: https://core.trac.wordpress.org/ticket/60851

#12 follow-up: @oglekler
2 months ago

@zodiac1978 can you take a look at two possible options I suggested? There is little time, but I think we still can finalize the solution or make an improvement this release and move further work to the next. I also don't like breakable layout and think that this should be fixed.

#13 in reply to: ↑ 12 @zodiac1978
2 months ago

Replying to oglekler:

@zodiac1978 can you take a look at two possible options I suggested? There is little time, but I think we still can finalize the solution or make an improvement this release and move further work to the next. I also don't like breakable layout and think that this should be fixed.

You only provided a screenshot for Bulk Edit. For me both version are still broken with using the "Quickedit" link.

It is working for the "Selects", but still broken for "Input" and the "Checkbox" is even more difficult. This should be next to the text ...

Last edited 2 months ago by zodiac1978 (previous) (diff)

@zodiac1978
2 months ago

Broken layout in Quickedit view

#14 @oglekler
2 months ago

@zodiac1978, sorry, I did only Bulk Edit to make my point that this can be fixed without special exceptions 🤷 There are two possible options to choose from, with their pros and cons. Grid is more flexible, but I am not sure that it's support is good enough for our case: https://caniuse.com/css-subgrid

@zodiac1978
2 months ago

Word break layout issue with CSS version 1

@zodiac1978
2 months ago

Word break layout issue with CSS version 1 and more issues (Quickedit)

#15 @zodiac1978
2 months ago

Let me sum it up:

Fiddling with markup and CSS will mean to punt this ticket as this will be not ready for 6.6 in time.

If it's too complex then something like the current patch is an OK solution for now, unless many more locales are affected. Keep in mind though it only works for de_DE, not de_AT or de_CH.

As de_AT and de_CH are copying from DE, every of those locales is affected:
de_DE, de_DE-formal, de_AT, de_CH, de_CH-informal

We could also try to use an attribute selector to shorten this up:
Something like body[class*="locale-de-"]

I would suggest using my approach for now and fix this better in 6.7 in a new ticket. This needs more time for research and discussion to fix this in a more modern and dynamic way.

@zodiac1978
2 months ago

Patch #2 targeting every de locale

#16 @oglekler
2 months ago

@swissspidy what do you think? I don't like an idea of temporarily fixes, but this one is quite simple. I have doubts if this is an enhancement as well, because it looks like a bug fix.

#17 @swissspidy
2 months ago

  • Keywords dev-feedback 2nd-opinion removed
  • Milestone changed from 6.7 to 6.6
  • Owner set to swissspidy
  • Status changed from new to reviewing
  • Type changed from enhancement to defect (bug)

I think it's fine to fix this like this for now.

#18 @swissspidy
2 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 58325:

Quick/Bulk Edit: Adjust label width to accommodate longer translations.

Specifically handles de_* locales.

Previously: [33598] / #33212.

Props zodiac1978, oglekler.
Fixes #60851.

@zodiac1978
2 months ago

Fixing incorrect patch after some wrong copy&paste

#19 @zodiac1978
2 months ago

@swissspidy I don't know if I need to open a new ticket or if this could be handled here.

I copied the same selector twice and missed some (compared to the same fix for ru_RU locale).

60851.3.diff fixes both.

There is still one negligible error:

With our patch we override .quick-edit-row-post fieldset.inline-edit-col-right label span.title which has a width set to auto and this is now set to 7em. But as this creates no layout error, I think it could stay this way.

#20 @zodiac1978
2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@zodiac1978
2 months ago

Now mobile fix also included (like for ru_RU and lt_LT)

#21 @zodiac1978
2 months ago

We could also fix the last mentioned layout issue with this code:

body[class*="locale-de-"] .quick-edit-row-post fieldset.inline-edit-col-right label span.title,
.locale-ru-ru .quick-edit-row-post fieldset.inline-edit-col-right label span.title,
.locale-lt-lt .quick-edit-row-post fieldset.inline-edit-col-right label span.title {
	width: auto;
}

@zodiac1978
2 months ago

Additionally fix some minor layout issue for the quickedit status label

#22 @swissspidy
8 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 58411:

Quick/Bulk Edit: Further adjust label width for translations. after [58325]

Props zodiac1978.
Fixes #60851.

Note: See TracTickets for help on using tickets.