Support from Acquia helps fund testing for Drupal Acquia logo

Comments

munzirtaha’s picture

Assigned: munzirtaha » Unassigned
Status: Active » Needs review
FileSize
879 bytes

Attached is a patch that fixed the issue

nathanlawsn’s picture

Patch in #1 fixes this issue.

Tags LTR Before:

Tags LTR before patch

Tags RTL Before:

Tags RTL before patch

Tags LTR After:

Tags LTR after patch

Tags RTL After:

Tags RTL after patch

nathanlawsn’s picture

Status: Needs review » Reviewed & tested by the community
LewisNyman’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/themes/bartik/css/components/content.css
@@ -122,6 +122,10 @@ h1#page-title {
 }
+/* This is required to win over specifity of .region-content ul */

In our CSS standards, we always have a blank line before every comment. See: https://www.drupal.org/node/1887862

nathanlawsn’s picture

nathanlawsn’s picture

Status: Needs work » Needs review
Eski’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
17.98 KB

Patch in #5 looks good and meets the coding standards, as mentioned in #4.

Patch Screenshot

Manjit.Singh’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs screenshots

@Eski Need some more screenshots including LTR after #5 patch to avoid regression issues.

Maninders’s picture

Assigned: Unassigned » Maninders
Status: Needs work » Active
Maninders’s picture

Assigned: Maninders » Unassigned
Status: Active » Needs review
Issue tags: -Needs screenshots
FileSize
17.3 KB
17.34 KB
596 bytes

I applied a patch #5, but there was an issue in rtl.
Please check screenshot of rtl-before.jpg.
And i have resolved that issue please check screenshot rtl-after.jpg.
And review patch.

jim005’s picture

We're working on at DrupalCon Barcelona at the French team table ;-)

Anonymous’s picture

I notice that the previous patchs change something in content.css but this file doesn't exist anymore.
We find the problem in the main-content.css so there is a new patch to review.

Thanks to Knee-X with my mentor Chipway during DrupalCon Barcelona

Anonymous’s picture

FileSize
30.62 KB

Here's a screenshot showing that the patch works.

jim005’s picture

Status: Needs review » Reviewed & tested by the community

Test passed the patch looks ready to go RTBC. ;-)

Status: Reviewed & tested by the community » Needs work

biguzis’s picture

Status: Needs work » Reviewed & tested by the community

Retested, got zero fails. Set to RTBC.

herom’s picture

Unfortunately, the patch in #12 isn't in the right direction. it breaks [dir="rtl"] .region-content ul in other places.

Here is the actual problem (taken from firebug):

An RTL rule overriding an unrelated rule, due to more specificity.

I've posted a more proper fix (in line with #10 and older patches), and a screenshot for after the patch.

herom’s picture

Status: Reviewed & tested by the community » Needs review
NikitaJain’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
146.74 KB
146.2 KB
144.17 KB
143.17 KB

Tested the 2509390-18.patch on Firefox and chrome browser for Ubuntu 14.04 and its working fine.
Padding removed from RTL mode after applying the patch.
Screenshot attached.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 6d7ce14 and pushed to 8.0.x and 8,1.x. Thanks!

  • alexpott committed 1d9f91e on 8.1.x
    Issue #2509390 by nathanlawson91, Maninders, herom, munzirtaha,...

  • alexpott committed 6d7ce14 on
    Issue #2509390 by nathanlawson91, Maninders, herom, munzirtaha,...

Status: Fixed » Closed (fixed)

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