Problem/Motivation

Enable locale, head to admin/config/regional/translate, and notice that the filter button has more distance between it and other elements than seems necessary.

This may impact other forms, this is just where I happened to notice it.

Proposed resolution

  • Get agreement that this should be changed
  • If there is agreement to change, determine the change that should be made
  • Determine if this impacts other forms
  • Impement, check for side effects in other forms

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#28 BeforeMR-Desktop.png54.48 KBkanchan bhogade
#28 BeforeMR-Mobile.png39.81 KBkanchan bhogade
#28 AfterMR-7841-desktop.png56.02 KBkanchan bhogade
#28 AfterMR-7841-Mobile.png43.28 KBkanchan bhogade
#27 afterFixD11.png79.6 KBdivya.sejekan
#27 afterFixD10.png82.8 KBdivya.sejekan
#27 beforefix3.png110.12 KBdivya.sejekan
#27 BeforeFix2.png126.34 KBdivya.sejekan
#27 beforeFix1.png62.12 KBdivya.sejekan
#21 3159892-21.patch495 bytesgauravvvv
#11 3159892-11.patch501 bytesphenaproxima
#11 3159892-11-after.png41.55 KBphenaproxima
#8 After-patch.png120.47 KBsakthivel m
#8 before-patch.png102.13 KBsakthivel m
#6 3159892-applied_patch.png17.81 KBabhijith s
#6 3159892-before.png18.15 KBabhijith s
#4 selected_9645.png102.46 KBKondratievaS
#3 3159892-3.patch2 KBkostyashupenko
Screen Shot 2020-07-17 at 11.03.23 AM.png41.42 KBbnjmnm

Issue fork drupal-3159892

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

mayurjadhav’s picture

mayurjadhav commented

Hi @bnjmnm
Yes we need to change this, Extra padding is added to the filter form action button on Locale translate filter form.
This is completely independent and removing padding will not make any impact on any other forms or action buttons.

.locale-translate-filter-form .form-actions {
    float: left;
    padding: 3.8ex 0 0 0;
}

Patch is ready with me, once we get the confirmation i'll update here.

Thanks.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Active » Needs review
StatusFileSize
new 3159892-3.patch2 KB

Not sure, maybe best way here to not just extend original library, but carefully rewrite it from scratch (from Claro only)?

KondratievaS’s picture

KondratievaS commented
StatusFileSize
new selected_9645.png102.46 KB

Tested patch from #3 for mobile and desktop resolutions in Chrome, FF, Safari and IE browsers. Result is OK

OK

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

abhijith s’s picture

abhijith s commented
StatusFileSize
new 3159892-before.png18.15 KB
new 3159892-applied_patch.png17.81 KB

Applied the patch #3 on 9.2.x and it works fine.The whitespace is reduced after this patch.Adding screenshots below.

Before patch;
before

After patch:
after

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sakthivel m’s picture

sakthivel m commented
StatusFileSize
new before-patch.png102.13 KB
new After-patch.png120.47 KB

Applied the patch #3 on 9.3.x and it works fine.The whitespace is reduced after this patch. Adding screenshots below.

Moved to RTBC

sakthivel m’s picture

sakthivel m commented
Status: Needs review » Reviewed & tested by the community
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work

Did you try if unloading core/modules/locale/css/locale.admin.css would solve this? I checked the CSS and it seems like at least most of it isn't needed in Claro.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 3159892-11-after.png41.55 KB
new 3159892-11.patch501 bytes

I tried @lauriii's suggestion of removing locale.admin.css in Claro, and it had the desired effect:

"After" screenshot, with spacing corrected

Attaching a patch which takes this approach.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

Good suggestion @lauriii, indeed the core locale.admin.css has a lot of unnecessary code, and I don't see any visual regressions after unloading it, only the desired improvements.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work

It seems like the patch from #11 failed CI.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Reviewed & tested by the community

Looks like it's passed now...?

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 3159892-11.patch, failed testing. View results

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Reviewed & tested by the community

Another unrelated test failure switched this out of RTBC. Switching back

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 3159892-11.patch, failed testing. View results

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review
StatusFileSize
new 3159892-21.patch495 bytes

Re-rolled Patch #11, as it's no longer applies to 10.1.x.

ameymudras’s picture

ameymudras commented
Status: Needs review » Reviewed & tested by the community

Ive tested the patch on 10.1.x and following are my findings

1. The Issues summary is clear and explains the problem
2. Was able to replicate the problem
3. The patch applies cleanly
4. The patch fixes the issue and eliminates the extra space.
5. Did a code review and its a simple change to offload the locale.admin.css css as per #10

Moving the issue to RTBC, skipping before / after screenshots, since a few have been already provided

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work

I am not sure we should actually get rid of this CSS file completely. The file seems to contain several UX enhancements to the translation related UIs. They may seem minor but I assume some thought has been put behind them.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Gauravvvv opened merge request !7841

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review

The updated solution as per #23 no longer removes the locale.admin.css file. Instead, it adds `padding-block-start: 0;` for the button. Please review

divya.sejekan’s picture

divya.sejekan commented
StatusFileSize
new beforeFix1.png62.12 KB
new BeforeFix2.png126.34 KB
new beforefix3.png110.12 KB
new afterFixD10.png82.8 KB
new afterFixD11.png79.6 KB

Verified for both D10 and D11 . The fix looks fine.
Verified using MR - 3159892-filter-button-in

Testing steps :
1. Enable language module
2. Navigate to /admin/config/regional/translate
3. Check the filter button

RTBC ++
Keeping in Review state for further review

kanchan bhogade’s picture

kanchan bhogade commented
StatusFileSize
new AfterMR-7841-Mobile.png43.28 KB
new AfterMR-7841-desktop.png56.02 KB
new BeforeMR-Mobile.png39.81 KB
new BeforeMR-Desktop.png54.48 KB

Hi
Tested MR !7841 on Drupal 11
MR is applied Cleanly...

Test Result:
The Filter Button extra whitespace reduced and it looks good now.

RTBC+1

Adding screenshots for reference

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Looking just at the issue summary

  • Get agreement that this should be changed

    • = can it be pointed out where the agreement was

  • If there is agreement to change, determine the change that should be made

    • = same

  • Determine if this impacts other forms

    • = was this done? Definitely don't see

  • Impement, check for side effects in other forms

    • = related to above

    Version: 11.x-dev » main

    Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

    Read more in the announcement.