Problem/Motivation

We spent an awful lot of time to get this page right, so its sad to see it in this state :/ The content language admin page is seriously misaligned if you only have Language module enabled and not yet Content translation module. The reason is the combination of the following:

  1. The content language page adds the language admin library but the library does not have the CSS for the page.
  2. The library name is not using the "drupal." prefix, so the added library is in fact not found (even if it would have the CSS).

Additionally to this, the CSS has bold text styling for a table column which only contains <label> elements, which in core are already bold, so these styles are superfluous. It is enough to set the widths. The problem illustrated:

Proposed resolution

  1. Rename the library so it conforms to the standard used by the rest of the files.
  2. Add the CSS file to the library.
  3. Fix the reference to the library at the other usage.
  4. Remove the superfluous bold styling.

Remaining tasks

Review. Commit.

User interface changes

The page is beautifully aligned:

API changes

None.

CommentFileSizeAuthor
#1 2315885-language-libraries-yml-1.patch2.11 KBgábor hojtsy
Content language | drupal8.local 2014-08-05 17-38-09 2014-08-05 17-38-22.png237.93 KBgábor hojtsy
Content language | drupal8.local 2014-08-05 17-39-38 2014-08-05 17-39-54.png255.37 KBgábor hojtsy

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2315885-language-libraries-yml-1.patch2.11 KB

Patch implemented as explained in the issue summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Reproduced with/without the patch at simplytest.me, and worked as described.

The css removed is redundant as "label" is already font-weight:bold looking with inspector.

I was not sure about the library name, because I haven't seen any like drupal.*.*. Found that "drupal.dialog.ajax" already exists on core, so must be fine.

RTBC for me. Thanks for finding it!

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Looks like the naming convention for libraries, just the first few ones found:

core/modules/block/block.libraries.yml:drupal.block:
core/modules/block/block.libraries.yml:drupal.block.admin:
core/modules/book/book.libraries.yml:drupal.book:
core/modules/ckeditor/ckeditor.libraries.yml:drupal.ckeditor:
core/modules/ckeditor/ckeditor.libraries.yml:drupal.ckeditor.plugins.drupalimagecaption:
core/modules/ckeditor/ckeditor.libraries.yml:drupal.ckeditor.admin:
core/modules/ckeditor/ckeditor.libraries.yml:drupal.ckeditor.drupalimage.admin:
core/modules/ckeditor/ckeditor.libraries.yml:drupal.ckeditor.stylescombo.admin:
core/modules/config_translation/config_translation.libraries.yml:drupal.config_translation.admin:
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Also note I change the library name AND a reference to the library, but there were 2 references to the library and the one I did not touch already used the drupal. prefixed name which is why it did not need to be touched. In ContentLanguageSettingsForm::buildForm(). So it looks like I rename the library, but in fact there is already a "correct" reference to the right name.

lewisnyman’s picture

lewisnyman
he/him
commented

In #2017257: Create generic layout classes we created some generic classes that can be reused to layout elements on the page instead of writing the CSS again and again. it looks like almost all of this CSS is for layout, do you think we could use those classes instead?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@LewisNyman: well, this one is fixing bugs in core. I am not sure how that layout column.quarter class work with table cells due to the media class that is applied. Would the table sizing change entirely on smaller screens? Not sure it is feasible to put that in scope here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Let's do the quickfix and we can improve in followups.

Committed f6b7dd3 and pushed to 8.0.x. Thanks!

  • alexpott committed f6b7dd3 on 8.0.x 
    Issue #2315885 by Gábor Hojtsy: Fixed Quickfix: language settings page...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -sprint

Yay, thanks.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@alexpott created a quick patch to find other cases of incorrect library (which nothing protects against in core). That found a menu_ui issue, see #2316459: Menu ui module refers to nonexistent library, lacks menu parent validation on content type settings for that one, I just provided a complete solution.

Status: Fixed » Closed (fixed)

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