Problem/Motivation

Drupal Core Status report page content overflows its background for small screen (mobile/tab).
Admin page/path : admin/reports/status

one

And this is because of single big-word issue. Here cron return as a single big-word.

one

Proposed resolution

The table CSS generate here from core/themes/seven/css/components/tables.css
and in line 55

td {
  padding: 10px 12px;
  text-align: left; /* LTR */
}

If we modify this CSS to 

td {
  padding: 10px 12px;
  text-align: left; /* LTR */
  word-break: break-all;
}

Then everything works perfectly in desktop/tab/mobile.

one

CommentFileSizeAuthor
#39 interdiff-2781031-32-39.txt312 bytesstarshaped
#39 2781031-39.patch1.69 KBstarshaped
#34 2781031-34-cron-link-mobile.png143.89 KBstarshaped
#32 2781031-32.patch1.29 KBstarshaped
#27 drupal-status-report-2781031-comment-27.patch1.56 KBjoginderpc
#24 drupal-2781031-24.patch1.99 KBalioso
#17 test_solution.png29.73 KBchernous_dn
#15 solution.jpg31.68 KBchernous_dn
#15 test-comment-14.png18.51 KBchernous_dn
#14 Status-report.png60.38 KBbandanasharma
#12 2781031-status-report-2.png57.56 KBstar-szr
#12 2781031-status-report-1.png180.55 KBstar-szr
#10 after_apply_patch(chrome).png47.54 KBbrahmjeet789
#10 after_apply_patch(ie).png46.81 KBbrahmjeet789
#10 after_applypatch.png29.94 KBbrahmjeet789
#10 befora_applypatch.png26.43 KBbrahmjeet789
#9 css_fixed-2781031-2.patch1.68 KBchanchal2002
#2 css_fixed-2781031-1.patch385 byteschanchal2002
report-page-css-fixed.png54.03 KBchanchal2002
report-page-css-error-cron.png91.93 KBchanchal2002
report-page-css-error.png54.39 KBchanchal2002

Comments

chanchal2002 created an issue. See original summary.

chanchal2002’s picture

chanchal2002 commented
StatusFileSize
new css_fixed-2781031-1.patch385 bytes

Please check and test the patch file.

chanchal2002’s picture

chanchal2002 commented
Issue summary: View changes
chanchal2002’s picture

chanchal2002 commented
Issue summary: View changes
chanchal2002’s picture

chanchal2002 commented
Issue summary: View changes
manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Issue tags: +frontend
swentel’s picture

swentel commented
Status: Active » Needs review

If you have a patch, you should set it to 'Needs review' so the testbot can run and other people can review the patch.
(I don't necessarily think this is major though, but keeping it on major will definitely get more visibility initially)

star-szr’s picture

star-szr
he/him
Primary language English
commented
Priority: Major » Normal

Thanks for the patch and report @chanchal2002.

I'm worried that applying this styling to all table cells in Seven will likely have unintended consequences.

Downgrading to normal because it doesn't seem major to me either.

chanchal2002’s picture

chanchal2002 commented
StatusFileSize
new css_fixed-2781031-2.patch1.68 KB

Thanks @swentel

hello @Cottser

If we apply this css for report page only, is this will be better approach !!!

Like, if we add new class system-status-report__status-description to the template file
core/themes/stable/templates/admin/status-report.html.twig in line no 30 and
core/modules/system/templates/status-report.html.twig in line no 32

     <td class="system-status-report__status-description">
        {{ requirement.value }}
        {% if requirement.description %}
          <div class="description">{{ requirement.description }}</div>
        {% endif %}
      </td>

and added new CSS in file core/themes/stable/css/system/system.admin.css

.system-status-report__status-description {
  word-break: break-all;
}

Working perfectly for me.

Please check and test the new patch css_fixed-2781031-2.patch

brahmjeet789’s picture

brahmjeet789 commented
StatusFileSize
new befora_applypatch.png26.43 KB
new after_applypatch.png29.94 KB
new after_apply_patch(ie).png46.81 KB
new after_apply_patch(chrome).png47.54 KB

@chanchal2002 your patch looks fine, i put your patch and it is working fine for me, i have tested on different different browsers on windows like internet explorer,firefox and chrome. Please find attachment for the same.

brahmjeet789’s picture

brahmjeet789 commented
Status: Needs review » Reviewed & tested by the community
star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2781031-status-report-1.png180.55 KB
new 2781031-status-report-2.png57.56 KB

I'm not really sure this is the way to go because it breaks the wording of the other rows. So although it fits on screen it seems like it degrades the usability and readability. Attaching a couple screenshots to demonstrate. This is making me reconsider #2765957: Core installation page content overflows its background (CSS bug) a bit too…

I think this needs further discussion. Perhaps we should only apply this to the cron row or something similar (probably best to do so via a markup manipulation of some kind so that contributed modules could use the same method).

star-szr’s picture

star-szr
he/him
Primary language English
commented
Related issues: +#2765957: Core installation page content overflows its background (CSS bug)
bandanasharma’s picture

bandanasharma commented
StatusFileSize
new Status-report.png60.38 KB

@cottser Can we increase the width of the class?
.system-status-report__status-title {
width: 40%; /* right now it is width: 25%; */
}
and also add css for class
.system-status-report {
table-layout: fixed;
word-wrap: break-word;
}
After making these changes, please find the attached screenshot which contains the output.

chernous_dn’s picture

chernous_dn commented
StatusFileSize
new test-comment-14.png18.51 KB
new solution.jpg31.68 KB

Hi @bandanasharma it is bad solution for small mobile device. Attach screenshot. May be we need update style for mobile, title and text width: 100% Attach screenshot.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Status: Needs review » Needs work
Issue tags: +mobile

@Chernous_dn: Can you create the one screenshot for real status report page ? , As you have created solutions.jpg
From that, i think we can take any decision, Because we have the real values then.

chernous_dn’s picture

chernous_dn commented
StatusFileSize
new test_solution.png29.73 KB

Hi @Manjit.Singh Yes sure. Attach screenshot.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

Thanks.
This looks like a clean way to represent the data. I liked it.

bandanasharma’s picture

bandanasharma commented

@Chernous_dn agree on your point but for solution.png, we will need to change the structure of the page.
Could you please provide code snippet for test_solution.png.

Status report page work is already in progress .
Should we postpone the issue till #665790:Redesign the status report page is completed?

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Status: Needs work » Postponed
Related issues: +#665790: Redesign the status report page

thanks a lot @bandanasharma, i was not aware of this related issue. Lets postponed this until #665790: Redesign the status report page got committed.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tompagabor’s picture

tompagabor commented
Status: Postponed » Needs work

Since #665790: Redesign the status report page has been commited, we can work on this again.

alioso’s picture

alioso commented
StatusFileSize
new drupal-2781031-24.patch1.99 KB

The new layout places the manual cron link outside a table at /admin/config/system/cron but the problem remains.

I modified the markup and added a system css file to target the link specifically which solves the issue

alioso’s picture

alioso commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 24: drupal-2781031-24.patch, failed testing.

joginderpc’s picture

joginderpc
Location Mohali
commented
StatusFileSize
new drupal-status-report-2781031-comment-27.patch1.56 KB

@alioso i had updated the files because previous patch was failed, please check this.

joginderpc’s picture

joginderpc
Location Mohali
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 27: drupal-status-report-2781031-comment-27.patch, failed testing.

joginderpc’s picture

joginderpc
Location Mohali
commented
Status: Needs work » Needs review
alioso’s picture

alioso commented

@joginderpc seems like your fix is failing as well. Not quite sure i understand what the report is spitting out

starshaped’s picture

starshaped
she/her
Primary language English
commented
StatusFileSize
new 2781031-32.patch1.29 KB

After discussing this with @phenaproxima and @xjm, I've moved the css to add the word breaks to system.admin.css instead of creating a whole new css file. The test was failing because it was checking that all of system's files were being overridden by Stable, which will not happen because of the backwards compatibility issue.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue tags: +Needs screenshots
starshaped’s picture

starshaped
she/her
Primary language English
commented
Version: 8.3.x-dev » 8.4.x-dev
Issue tags: -Needs screenshots
StatusFileSize
new 2781031-34-cron-link-mobile.png143.89 KB

Attaching screenshot of the cron link with the word break css applied in a mobile device.

starshaped’s picture

starshaped
she/her
Primary language English
commented
Issue tags: +DrupalCampMontréal2017
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue tags: +Needs subsystem maintainer review

The code in the patch looks perfect.

However...I'm not clear on front-end policy but it is my understanding (quite probably flawed) that we can't change things in Stable? So I'm not certain the patch in #32 can be committed as-is because it changes Stable. I think a front-end maintainer should weigh in here, so I'm tagging this for subsystem maintainer review.

joginderpc’s picture

joginderpc
Location Mohali
commented
Status: Needs review » Reviewed & tested by the community
star-szr’s picture

star-szr
he/him
Primary language English
commented
Title: Core Status report page content overflows its background for mobile/tab (CSS bug) » Cron link on cron page overflows its background on smaller screens
Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs subsystem maintainer review

With Stable and Classy, things can be changed if the benefit outweighs the potential risks, and I think that is absolutely the case here. The chance that this change will break things for people is very low and this change will make the page not look broken, yay! :)

I would also consider this a purely additive change since it's styling a brand new class.

However, this change needs to be made in system.admin.css in the core system module as well, to ensure that this change is carried forward and will also work for people not using Stable/Classy as a base.

starshaped’s picture

starshaped
she/her
Primary language English
commented
StatusFileSize
new 2781031-39.patch1.69 KB
new interdiff-2781031-32-39.txt312 bytes

Added this change to the core system module's system.admin.css file.

starshaped’s picture

starshaped
she/her
Primary language English
commented
Status: Needs work » Needs review
phenaproxima’s picture

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

Looks like @Cottser's feedback is addressed, so back to RTBC you go.

  • Cottser committed 8434c1a on 8.4.x 
    Issue #2781031 by chanchal2002, starshaped, joginderpc, alioso,...
star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 8434c1a and pushed to 8.4.x. Thanks all!

Status: Fixed » Closed (fixed)

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