Follow-up to #2565123: Move logic from SafeMarkup::format() from template_preprocess_locale_translation_update_info() to template

Problem/Motivation

The parent issue added a bit of a strange item_list class, which is confusingly similar to item-list AND on the child element this time (ul instead of parent div).

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task, nothing is broken.
Issue priority Normal, just a small bit of follow-up.
Unfrozen changes Unfrozen because it only changes markup. (Which? Specify.)
Disruption Not disruptive at all.

Proposed resolution

Remove the unneeded class. The CSS in that issue does not use this selector. Also checked with ack --css item_list.

Remaining tasks

  1. Create Patch
  2. Review
  3. Commit

Steps to Reproduce

  1. Enable Interface Translation & lanaguage
  2. Enable one other language
  3. Go to Reports admin/reports/translations

User interface changes

n/a

API changes

n/a

Files: 
CommentFileSizeAuthor
#6 followup_remove-2571553-6.patch682 bytesJmdrawneek@googlemail.com
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,721 pass(es), 1 fail(s), and 1 exception(s). View
#3 followup_remove-2571553-3.patch2 KBJmdrawneek@googlemail.com
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,597 pass(es). View

Comments

Cottser created an issue. See original summary.

Cottser’s picture

Component: base system » locale.module
Jmdrawneek@googlemail.com’s picture

Assigned: Unassigned » Jmdrawneek@googlemail.com
Status: Active » Needs review
FileSize
2 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,597 pass(es). View

This is my first commit! I've removed the class but also spotted the language label field needed some padding to match the column heading.

lauriii’s picture

Status: Needs review » Needs work
diff --git a/core/modules/locale/css/locale.admin.css b/core/modules/locale/css/locale.admin.css
index 0d1e42e..0bd0b80 100644
--- a/core/modules/locale/css/locale.admin.css
+++ b/core/modules/locale/css/locale.admin.css
@@ -39,23 +39,18 @@
 .locale-translate-edit-form td {
   vertical-align: top
 }
-
 .locale-translate-edit-form tr.changed {
   background: #ffb;
 }
-
 .locale-translate-edit-form tr .form-type-item .ajax-changed {
   position: absolute;
 }
-
 .locale-translate-filter-form .form-wrapper {
   margin-bottom:0;
 }
-
 .locale-translate-edit-form table.changed {
   margin-top: 0;
 }
-
 /**
  * Available translation updates page.
  */
@@ -68,8 +63,6 @@
 #locale-translation-status-form th.title {
   width: 25%;
 }
-#locale-translation-status-form th.description {
-}
 #locale-translation-status-form td {
   vertical-align: top;
 }
@@ -81,11 +74,13 @@
 .expanded .locale-translation-update__wrapper {
   background: transparent url(../../../misc/menu-expanded.png) left .6em no-repeat;
 }
-
...
 #locale-translation-status-form .description {
   cursor: pointer;
...
-}
+}
\ No newline at end of file

Thanks for the patch but TBH I think these changes are unrelated

Cottser’s picture

Assigned: Jmdrawneek@googlemail.com » Unassigned

Thanks @jmdrawneek!

Please remove the CSS changes, that sounds like a separate issue.

Jmdrawneek@googlemail.com’s picture

FileSize
682 bytes
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,721 pass(es), 1 fail(s), and 1 exception(s). View

Ok sorry - thought I was being helpful.

lauriii’s picture

Status: Needs work » Needs review
Issue tags: +Needs screenshots

No problem at all and thanks for the new version of the patch! If you feel like something needs to be fixed, you are free to open up a new issue for it. Usually we try to keep the scope of single issue as tight as possible to get things done nice and quick :)

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs screenshots

Doesn't need screenshots. That class only effects uls and li's on the div wrapper (sorry my silly mistake).

@jmdrawneek thank you for helping fix my mistakes:) You are helpful for finding that other problem with the UI. Could you open an issue if one doesn't exist with screenshots there showing the problem and your CSS solution to that problem?

joelpittet’s picture

Issue tags: +Needs followup

Needs follow-up for CSS issue identified #3

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: followup_remove-2571553-6.patch, failed testing.

Cottser’s picture

Status: Needs work » Reviewed & tested by the community

Grumpy old testbot.

GET http://ec2-52-11-109-200.us-west-2.compute.amazonaws.com/checkout/admin/modules/uninstall returned 0 (0 bytes).

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 877fe96 and pushed to 8.0.x. Thanks!

  • alexpott committed 877fe96 on 8.0.x
    Issue #2571553 by Jmdrawneek@googlemail.com, Cottser: Followup: Remove...
Jmdrawneek@googlemail.com’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.