Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.According to bartik.breakpoints.yml the breakpoint where mobile goes into narrow is 560px:
bartik.mobile:
label: mobile
mediaQuery: ''
weight: 0
multipliers:
- 1x
bartik.narrow:
label: narrow
mediaQuery: 'all and (min-width: 560px) and (max-width: 850px)'
weight: 1
multipliers:
- 1x
bartik.wide:
label: wide
mediaQuery: 'all and (min-width: 851px)'
weight: 2
multipliers:
- 1x
Yet, the CSS media queries for the "hamburger menu" state 460px:
diff --git a/docroot/core/themes/bartik/css/components/header.css b/docroot/core/themes/bartik/css/components/header.css
index 40ea0f0..9ec9ce2 100644
--- a/docroot/core/themes/bartik/css/components/header.css
+++ b/docroot/core/themes/bartik/css/components/header.css
@@ -11,7 +11,7 @@
.region-header .site-branding {
margin-top: 0.429em;
}
-@media all and (min-width: 461px) {
+@media all and (min-width: 561px) {
.region-header .block {
float: right; /* LTR */
margin-top: 0.357em;
diff --git a/docroot/core/themes/bartik/css/components/primary-menu.css b/docroot/core/themes/bartik/css/components/primary-menu.css
index 1132ae2..7aa15e3 100644
--- a/docroot/core/themes/bartik/css/components/primary-menu.css
+++ b/docroot/core/themes/bartik/css/components/primary-menu.css
@@ -111,7 +111,7 @@ body:not(:target) .region-primary-menu .menu-toggle-target-show:target ~ .menu .
/**
* Media queries for primary menu.
*/
-@media all and (min-width: 461px) and (max-width: 900px) {
+@media all and (min-width: 561px) and (max-width: 900px) {
.region-primary-menu .menu {
margin: 0 5px;
padding: 0;
@@ -203,9 +203,9 @@ body:not(:target) .region-primary-menu .menu-toggle-target-show:target ~ .menu .
/**
* Ensures that the open mobile menu hides when the screen dimensions become
- * 461px or wider.
+ * 561px or wider.
*/
-@media all and (min-width: 461px) {
+@media all and (min-width: 561px) {
body:not(:target) .region-primary-menu .menu-toggle-target-show:target ~ .menu-toggle--hide {
display: none;
}
diff --git a/docroot/core/themes/bartik/css/components/site-branding.css b/docroot/core/themes/bartik/css/components/site-branding.css
index fda5d9e..cc947a8 100644
--- a/docroot/core/themes/bartik/css/components/site-branding.css
+++ b/docroot/core/themes/bartik/css/components/site-branding.css
@@ -16,7 +16,7 @@
display: inline-block;
vertical-align: top;
}
-@media all and (min-width: 461px) {
+@media all and (min-width: 561px) {
.site-branding__text {
margin-bottom: 1.857em;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #38 | 2868632--after--patch--pic.png | 88.42 KB | vikashsoni |
| #38 | 2868632--after--patch--pic2.png | 87.66 KB | vikashsoni |
| #38 | 2868632--before--patch--pic.png | 93.67 KB | vikashsoni |
| #27 | after-patch.png | 155.47 KB | Rinku Jacob 13 |
| #27 | before-patch.png | 154.63 KB | Rinku Jacob 13 |
Issue fork bartik-2868632
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:
changes
Comments
Comment #2
mpp CreditAttribution: mpp as a volunteer and at AmeXio commentedComment #3
bander2 CreditAttribution: bander2 as a volunteer commentedThanks for your contribution mpp!
This patch won't apply. It looks like the patch was created from outside the drupal installation directory.
The patch's name should also follow the patch naming convention.
Comment #4
mpp CreditAttribution: mpp as a volunteer and at AmeXio commentedNew patch against 8.4.x
Comment #5
bander2 CreditAttribution: bander2 as a volunteer commentedWorks great!
Comment #6
Gábor HojtsyThanks, looked at this.
First of all do these need to be in sync in the first place? I would love to see some git archeology on how we ended up at this place, which one was first, if this difference was discussed in the issues already when introducing the later change, etc?
Also if they do need to be in sync I think it would be "more backwards compatible" to fix this in the breakpoints YML file and not the CSS, no? The number of times this appears in the CSS makes it pretty apparent that was intentional.
That said I asked for a second opinion from @lauriii as well.
Comment #14
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commentedPlease review
Comment #15
djsagar CreditAttribution: djsagar at OpenSense Labs commentedPatch #14 is working fine in drupal9.2.
Thanks!
Comment #16
djsagar CreditAttribution: djsagar at OpenSense Labs commentedComment #17
lauriiiTagging for subsystem maintainer review based on #6.
Comment #18
longwaveI did the archaeology for this as requested in #6.
min-width: 461pxin CSS was first introduced in #1192044-73: Convert Bartik's layout to mobile-first and responsive. This even has a comment:Between #88 and #98 in that issue, the @todo was lost but there was no discussion about it. Another @todo about breakpoints remains but #113 notes that "i don't agree that agreeing on a breakpoint width is a prerequisite for committing this" and the patch is committed in #114.
The 461px value is then spread to multiple places in a number of later refactoring issues.
Separately,
min-width: 560pxin the breakpoints config was added in #1775774: Allow themes to identify their breakpoints to Drupal. In #36 Gabor stated "as per the intent of this issue, these breakpoints are just a static copy-paste from the breakpoints already defind in CSS to inform the rest of the system" but this must have not been checked that closely at the time!Comment #19
vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commentedthe issue is not still there and no need for apply patch. It's working fine and there is no breakpoints
Comment #20
Madhu kumar CreditAttribution: Madhu kumar as a volunteer and commentedPatch #14 applied cleanly and it is working well. Added screenshot for reference
Before patch:
After patch:
Comment #21
longwaveThanks to everyone who's taken screenshots.
Back to RTBC following #18 and the screenshots.
Comment #24
kiran.kadam911Thanks to everyone! Just re-rolling patch to 9.3.x
Kindly review.
Thanks!
Comment #25
Sakthivel M CreditAttribution: Sakthivel M at QED42 commented@ranjith_kumar_k_u thanks,
Verified and tested patch#24. Patch applied successfully and looks good to me.
Moving to RTBC
Comment #26
Sakthivel M CreditAttribution: Sakthivel M at QED42 commentedComment #27
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Zyxware Technologies commentedverified and tested patch #24.patch applied successfully for 9.3.x dev
Comment #29
Sakthivel M CreditAttribution: Sakthivel M at QED42 commentedComment #31
longwaveRandom fail, back to RTBC
Comment #33
longwaveRandom fail again.
Comment #35
vsujeetkumar CreditAttribution: vsujeetkumar at Srijan | A Material+ Company for Drupal India Association commentedRe-test on #24, Fixed the Random fail test, Now it is on Green, Move again to RTBC.
Comment #38
vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commentedApplied patch #24 in drupal-9.3.x-dev and applied successfully
After patch the issue has been fixed on mobile responsive on 560 port
Thanks for the patch
For ref sharing screenshot ...
Comment #39
longwaveRandom fail yet again, back to RTBC.
Comment #43
pameeela CreditAttribution: pameeela commentedMoving to contrib since Bartik was removed from core in D10.
Comment #45
djsagar CreditAttribution: djsagar at OpenSense Labs commentedCreated MR request based on #24, kindly review.
Thanks!