Problem/Motivation

New views grid look a little out of style in Bartik. We need to add some styling cleanup and responsive support for views-grids so they look/work properly.

Proposed resolution

Cleaning up the CSS

Remaining tasks

Manual review

User interface changes

Grid view CSS

API changes

None

Original report

The patch at #1903746-44: Replace the views grid table template with one using divs makes the new views grid look a little janky in Bartik. We need to add some styling cleanup and responsive support for views-grids so they look/work properly.

CommentFileSizeAuthor
#87 interdiff.txt839 byteslauriii
#87 2031447-87.patch1.58 KBlauriii
#84 after.png141.51 KBanmolgoyal74
#84 before.png140.79 KBanmolgoyal74
#81 afterPatch_mobileView2.png23.04 KBRuchi Joshi
#81 afterPatch_mobileView1.png20.71 KBRuchi Joshi
#81 afterPatch_verticalView.png38.64 KBRuchi Joshi
#81 afterPatch_horizontalView.png36.3 KBRuchi Joshi
#80 interdiff_77-79.txt1.77 KBkomalk
#79 2031447-79.patch1.66 KBkomalk
#77 2031447-77.patch1.69 KBSantosh_Verma
#77 interdiff_75-77.txt2.29 KBSantosh_Verma
#77 Grid-Vertical-view-Mobile.png315.8 KBSantosh_Verma
#77 Grid-Vertical-view.png589.4 KBSantosh_Verma
#77 Grid-Horizontal-view-Mobile.png294.58 KBSantosh_Verma
#77 Grid-Horizontal-view.png482.51 KBSantosh_Verma
#76 afterPatch_mobileView.png26.66 KBRuchi Joshi
#76 afterPatch_verticalView.png80.36 KBRuchi Joshi
#76 afterPatch_horizontalView.png56.7 KBRuchi Joshi
#75 interdiff_74-75.txt2 KBkomalk
#75 2031447-75.patch1.8 KBkomalk
#75 After-patch.png527.71 KBkomalk
#75 Before-patch.png494.87 KBkomalk
#74 2031447-74.patch1.95 KBkomalk
#59 grid_system.jpg320.2 KBpakmanlh
#58 bottom-row.png129.93 KBemma.maria
#58 article-view___Site-Install.png315.7 KBemma.maria
#57 interdiff-54-57.txt697 bytespakmanlh
#57 add_new_views_grid-2031447-57.patch1.95 KBpakmanlh
#55 0_0_body_2_tablet_h.png81.56 KBemma.maria
#54 add_new_views_grid-2031447-54.patch1.99 KBstephenrodrigo@yahoo.com
#53 add_new_views_grid-2031447-53.patch1.97 KBstephenrodrigo@yahoo.com
#53 desktop-verticle-gridview.png1.02 MBstephenrodrigo@yahoo.com
#51 mobile-horizontal-gridview.png811.4 KBstephenrodrigo@yahoo.com
#51 tab-vertical-gridview.png1.04 MBstephenrodrigo@yahoo.com
#51 tab-horizontal-gridview.png917.82 KBstephenrodrigo@yahoo.com
#51 desktop-verticle-grid.png1.09 MBstephenrodrigo@yahoo.com
#51 desktop-horizontal-gridview.png971.23 KBstephenrodrigo@yahoo.com
#48 add_new_views_grid-2031447-48.patch1.94 KBjorgeegomez
#43 2031447-add-new-views-grid-43.patch3.96 KBmherchel
#42 2031447-add-new-views-grid-42.patch4.05 KBmherchel
#39 interdiff-2031447-37-38.txt1.07 KBDickJohnson
#39 drupal-add-new-views-grid-2031447-38.patch2.33 KBDickJohnson
#39 pad-540px.png198.79 KBDickJohnson
#39 desktop-950px.png224.34 KBDickJohnson
#38 grit-ad-640px.png242.8 KBDickJohnson
#38 grid-ad-540.png232.32 KBDickJohnson
#37 interdiff-2031447-36-37.txt3.53 KBDickJohnson
#37 drupal-add-new-views-grid-2031447-37.patch2.49 KBDickJohnson
#36 drupal-add-new-views-grid-2031447-36.patch2.44 KBDickJohnson
#32 Screen Shot 2015-01-04 at 18.01.16.png190.97 KBemma.maria
#31 drupal-add-new-views-grid-2031447-31.patch2.01 KBDickJohnson
#28 desktop_screenshot.png149.33 KBteroelonen
#28 tablet_screenshot.jpg130.38 KBteroelonen
#28 mobile_horizontal_screenshot.jpg464.73 KBteroelonen
#28 mobile_screenshot.jpg271.12 KBteroelonen
#28 drupal-add-new-views-grid-2031447-28.patch2.12 KBteroelonen
#21 drupal-add-new-views-grid-2031447-21.patch2.19 KBlauriii
#13 test | localhost 2014-01-04 12-50-34.png114.57 KBbrahmjeet789
#8 drupal-add-new-views-grid-2031447-8.patch2.62 KBmarkhalliwell
#8 interdiff.txt1 KBmarkhalliwell
#4 drupal-add-new-views-grid-2031447-4.patch2.64 KBmarkhalliwell
#2 horizontal-grid.png397.3 KBmarkhalliwell
#2 vertical-grid.png424.89 KBmarkhalliwell
#1 drupal-add-new-views-grid-2031447-1.patch2.93 KBmarkhalliwell

Issue fork drupal-2031447

Command icon 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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markhalliwell’s picture

Status: Active » Needs review
FileSize
2.93 KB

Patch

markhalliwell’s picture

FileSize
424.89 KB
397.3 KB

Horizontal:

horizontal-grid.png

Vertical:

vertical-grid.png

dawehner’s picture

Issue tags: +VDC

Just in general they are looking great!

markhalliwell’s picture

Removed the first/last class selectors since they're no longer provided by the plugin

markhalliwell’s picture

joelpittet’s picture

Status: Needs review » Needs work
Issue tags: -Needs manual testing

Looks good, just a couple CSS nitpiks and I say ship it:)

+++ b/core/themes/bartik/css/style.css
@@ -1929,6 +1929,111 @@ div.admin-panel .description {
+  border-bottom: 0 none;
...
+  border-bottom: 0 none;
...
+  border-right: 0 none;
...
+    border: 0 none;

0 alone will suffice.

markhalliwell’s picture

Status: Needs work » Needs review
FileSize
1 KB
2.62 KB

Fixed very minor nitpick in #7.

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me and is fairly small bit of CSS to boot.

RTBC+1.

alexpott’s picture

Assigned: markhalliwell » JohnAlbin

Assigning to John Albin for a css review...

xjm’s picture

webchick’s picture

Status: Reviewed & tested by the community » Needs review

I think alexpott meant to move this down to "needs review."

I don't know that John Albin Himself™ needs to review this, but what we need is something a bit more firm like "I inspected the patch line by line, can confirm the approach taken here is the best one, checked it against the CSS/markup standards, and tested it in X browsers, and it looks good to me."

brahmjeet789’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
114.57 KB

I have applied the patch #8 its working fine for my machine .

klonos’s picture

Status: Reviewed & tested by the community » Needs review

@brahmjeet789: please see what Angie said back in #12 would qualify to set this issue to RTBC.

jibran’s picture

joelpittet’s picture

Issue tags: +CSS
LewisNyman’s picture

Status: Needs review » Needs work
  1. +++ b/core/themes/bartik/css/style.css
    @@ -1929,6 +1929,111 @@ div.admin-panel .description {
    +@media all and (max-width: 520px) {
    +  .views-view-grid .views-col {
    +    float: none;
    +    padding: 0;
    +    width: 100% !important;
    +  }
    

    Is there a good reason not to use mobile first media queries here?

  2. +++ b/core/themes/bartik/css/style.css
    @@ -1929,6 +1929,111 @@ div.admin-panel .description {
    +.views-view-grid.vertical .views-row {
    +  margin-top: 30px;
    +  margin-bottom: 30px;
    +}
    +
    +.views-view-grid.vertical .views-row:first-child {
    +  margin-top: 0;
    +}
    +
    +.views-view-grid.vertical .views-row:last-child {
    +  margin-bottom: 0;
    +}
    

    I can see few situations where we're adding styling just to undo it for first/last children.

    Can I suggest the adjacent sibling selector instead? It would reduce the code required a lot.

LewisNyman’s picture

Issue tags: +frontend
dawehner’s picture

Assigned: JohnAlbin » Unassigned

Not that I have any clue about css, but
width: 100% !important
really looks desperate, is this needed?

@LewisNyman
It would be great if you can find someone at drupalcon to work on this problem.

emma.maria’s picture

Assigned: Unassigned » emma.maria

On it :)

lauriii’s picture

Assigned: emma.maria » Unassigned
Status: Needs work » Needs review
FileSize
2.19 KB

Answer for #17/1 and #19: We need to use width: 100% !important because we have width 50% as inline CSS in the .views-col. If we would want something better, we need to change Views grid to use something else than inline CSS (classes?).

joelpittet’s picture

Thanks @lauriii for the patch and figuring out why we are using !important. Would you mind opening up an issue to move that inline styles to css classes? i don't think this needs to be dependant on that issue and the !important can go in, but it would be nice to also look at that approach on it's own... it may be hard with different grid sizes to come up with a set of class names that aren't bloating the views CSS but I do believe worth a try. What do you think?

Maybe 1-12 columns and anything more than that could dealt with custom CSS? Or an option to do inline styles with tokens based on the columns may be worth a look.

markhalliwell’s picture

Status: Needs review » Needs work

No, the !important is necessary because the views grid style plugin has an "Automatic width" setting which adds the dynamic width via PHP: http://cgit.drupalcode.org/drupal/tree/core/modules/views/views.theme.in...

A bug I just realized is that this would apply to grids that do not use that setting. It should also really reset to it's initial width (determined by element type), not just 100%.

So perhaps it should really be the following instead:

/**
 * !important is needed to override the "automatic widths" setting.
 */
.views-view-grid .views-col[style^="width:"],
.views-view-grid .views-col[style*=" width:"] {
  width: inherit !important;
}

Also, I know I created this (a long time ago), but it really should be mobile first with min breakpoints, not max. Cannot remember if it's because bartik wasn't mobile first (at the time) and I was just following existing code.

lauriii’s picture

Assigned: Unassigned » lauriii

Maybe 1-12 columns and anything more than that could dealt with custom CSS? Or an option to do inline styles with tokens based on the columns may be worth a look.

If I understood right, @joelpittet is talking about "Automatic width" setting in this comment. I opened new issue for that: #2294153: Use classes instead of inline CSS in Views automatic width option

@Mark Carver thank you for your input, I didn't know width: initial exists. With that setting it is possible to make it 100% mobile-first. I can keep working on this one.

markhalliwell’s picture

Sorry, that was a typo. It should be inherit, not initial (which isn't supported by IE).

lauriii’s picture

Assigned: lauriii » Unassigned

What do you think could we use min-width: 100% and min-width: 0 to override inline CSS? Then we wouldn't need any !important or non mobile first media queries.

markhalliwell’s picture

Yes, that could work.

teroelonen’s picture

Status: Needs work » Needs review
FileSize
2.12 KB
271.12 KB
464.73 KB
130.38 KB
149.33 KB

Added the min-width fix and seems to be working real well. Browser support for min-width.

Mobile screenshot:

Mobile horizontal screenshot:

Tablet screenshot:

Desktop screenshot:

emma.maria’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
emma.maria’s picture

This issue needs more work than just adding the code from the previous patch again.

Looking at the screenshots in #26. We have acquired a border bottom on the bottom of every Node teaser. This style is applied as follows...

.node--view-mode-teaser {
  border-bottom: 1px solid #d3d7d9;
  margin-bottom: 30px;
  padding-bottom: 15px;
}

By default this is great for the homepage view, where you get the standard view of most recent content, a nice border underneath each teaser. But, no matter where or how the teasers are displayed you get a border bottom which in my eyes is incorrect. If the user has views knowledge (and views is in core now) and wants to display a grid, or a teaser in the sidebar, triptych/tertiary region they will be faced with a bottom border.

So as part of using the ace work above by @Mark Carver we need to tackle the issue I raised above too.

DickJohnson’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.01 KB

All the patches have been written before the SMACSS split, so reroll is needed anyways. Starting with that one.

emma.maria’s picture

Status: Needs review » Needs work
FileSize
190.97 KB

This should not be set to Needs Review, the work to fix the design for grids needs to be completed too.

The styling for the lines under each teaser needs to be reworked so this doesn't happen...

We are aiming for the screenshots in #2

DickJohnson’s picture

Assigned: Unassigned » DickJohnson

The need review was just to get feedback from bot. Forgot to assign it to myself so assigning now.

DickJohnson’s picture

Assigned: DickJohnson » Unassigned
LewisNyman’s picture

  1. +++ b/core/themes/bartik/css/components/views.css
    @@ -109,3 +109,100 @@
    +.views-view-grid .views-col {
    + -moz-box-sizing: border-box;
    + -webkit-box-sizing: border-box;
    + box-sizing: border-box;
    + min-width: 100%;
    + padding: 0;
    +}
    +
    +.views-view-grid.horizontal .views-row,
    +.views-view-grid.vertical .views-row {
    + border-bottom: 1px solid #d3d7d9;
    + margin-bottom: 15px;
    + padding-bottom: 30px;
    + padding-top: 15px;
    +}
    

    These properties aren't indented. Also we don't need to include -moz- or -webkit- prefixes anymore

  2. +++ b/core/themes/bartik/css/components/views.css
    @@ -109,3 +109,100 @@
    +   min-width: 0;
    

    Min-width: 0? What does this do? You can't have less than 0 :P

  3. +++ b/core/themes/bartik/css/components/views.css
    @@ -109,3 +109,100 @@
    + }
    +
    + .views-view-grid .views-col:first-child {
    +   padding-left: 0;
    

    Only add blank links above comments please.

DickJohnson’s picture

Assigned: Unassigned » DickJohnson
FileSize
2.44 KB

Started to work on cleanup. I'm travelling to another part of country and changing computer so putting patch here to see test bot and to keep it safe. No review needed, will continue in few hours.

DickJohnson’s picture

Status: Needs work » Needs review
FileSize
2.49 KB
3.53 KB

Fixed some coding standard issues.

@lewisnyman asked in #35 why min-width: 0; is there. It's overriding min-width: 100%; that we're using to override inline width: 33.33333% and making the grid of 3 to actually work. There is a better solution for that and I'll take a look on it later today.

DickJohnson’s picture

FileSize
232.32 KB
242.8 KB

Before the breakpoint was written to 520px, I resized it to 540px but I think it's still too thin. As a attacment screenshot of 541px.

Grid at 540px

I think we should change the breakpoint to somewhere like 620px of 640px. As an attachment example of 640px.

Grid at 640px

DickJohnson’s picture

Now I think it's starting to be in relatively good shape.

Pad:
Pad

Desktop:
Desktop

DickJohnson’s picture

Assigned: DickJohnson » Unassigned

Unassigning. I didn't fix the min-width: 0; mentioned by @lewisnyman previously as I didn't find a good solution for that.

emma.maria’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll, +Novice
mherchel’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
4.05 KB

Rerolled. I also removed some extra linebreaks.

Can probably use some additional visual regression testing.

mherchel’s picture

Prior patch had a weird issue (encoding?). New patch attached. Still needs visual review.

joelpittet’s picture

Status: Needs review » Needs work

Ha, cross posted with you, thanks for cleaning that up:)

+++ b/core/themes/bartik/css/components/views.css
@@ -3,9 +3,8 @@
-  padding-left: 0; /* LTR */
+  padding-left: 0;
...
-

@@ -21,29 +20,24 @@
-  padding-left: 0; /* LTR */
+  padding-left: 0;
...
-
...
-
...
-
...
-
...
-

@@ -64,7 +58,6 @@
-

@@ -76,42 +69,113 @@
-
...
-
...
-  padding: 0 0 4px 2px; /* LTR */
+  padding: 0 0 4px 2px;
...
-
...
-
...
-
...
-
...
-  margin-right: 0; /* LTR */
+  margin-right: 0;

These line removal and dropping /*LTR*/ changes shouldn't be made.

joelpittet’s picture

+++ b/core/themes/bartik/css/components/views.css
@@ -76,42 +69,113 @@
+  .views-view-grid .node--view-mode-teaser h2 {

Is there a class we could target here? Would be nicer than trying to target a header at a specific level.

LewisNyman’s picture

Is there a class we could target here? Would be nicer than trying to target a header at a specific level.

hmm, node__title feels like a better class to use here.

emma.maria’s picture

Issue tags: +LatinAmerica2015
jorgeegomez’s picture

Status: Needs work » Needs review
FileSize
1.94 KB

Cleaned up mherchel's patch in #43, according to joelpittet and LewisNyman comments on #44 and #46

joelpittet’s picture

Issue tags: +Needs screenshots

Awesome thanks @jorgeegomez, just needs manual screenshots.

Super minor nitpick:

+++ b/core/themes/bartik/css/components/views.css
@@ -115,3 +115,74 @@
+  /* Vertical grids */

Needs a period ending the sentence.

stephenrodrigo@yahoo.com’s picture

Assigned: Unassigned » stephenrodrigo@yahoo.com
stephenrodrigo@yahoo.com’s picture

@joelpittet - I have attached screenshot from Chrome 40.0.2214.115.
In vertical grid view column percentage value breaking the structure because of the border.
Needs to add box-sizing: border-box to .views-view-grid.vertical .views-col

stephenrodrigo@yahoo.com’s picture

Status: Needs review » Needs work
stephenrodrigo@yahoo.com’s picture

Assigned: stephenrodrigo@yahoo.com » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs screenshots +Needs manual testing, +DrupalSouth
FileSize
1.02 MB
1.97 KB

I have actually patch the vertical grid view column structure.

stephenrodrigo@yahoo.com’s picture

I have missed the last curly bracket. It is fixed in this patch.

emma.maria’s picture

Issue summary: View changes
Status: Needs review » Needs work
FileSize
81.56 KB

I have tested the Views grid styling with 10 nodes at different screen widths.

Everything looks great apart from one thing.

Above 640px the padding to space out the teasers horizontally (which is applied to .views-view-grid .node--view-mode-teaser) causes the teasers to not be flush with the edges of the view container....
 

 
We need to move the padding from that level in the markup to the views-col level.

Code improvement suggestions

  1. +++ b/core/themes/bartik/css/components/views.css
    @@ -115,3 +115,85 @@
    +  .views-view-grid .node--view-mode-teaser {
    +    border-bottom: 0;
    +    margin: 0;
    +    padding: 0 15px;
    +  }
    

    Remove the padding from this level of the markup.

  2.  

  3. +++ b/core/themes/bartik/css/components/views.css
    @@ -115,3 +115,85 @@
    +  .views-view-grid .views-col {
    +    float: left;
    +    min-width: 0;
    +  }
    

    Add padding-right: 15px; and box-sizing: border-box; to this section.

  4.  

  5. +++ b/core/themes/bartik/css/components/views.css
    @@ -115,3 +115,85 @@
    +  .views-view-grid .views-col:first-child {
    +    padding-left: 0;
    +  }
    +
    +  .views-view-grid .views-col:last-child {
    +    padding-right: 0;
    +  }
    

    Keep the current last-child styling but remove the first-child as it is not needed.

pakmanlh’s picture

Assigned: Unassigned » pakmanlh

Workin on it.

pakmanlh’s picture

Assigned: pakmanlh » Unassigned
Status: Needs work » Needs review
FileSize
1.95 KB
697 bytes

I attached a new patch following the #55 instructions.

emma.maria’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +drupaldevdays
FileSize
315.7 KB
129.93 KB

Removing the padding at the beginning and end of the row makes the view look much better.
However of course adding padding to all rows apart from the last means the last row is a bit wider than the others.
 

 

I am not so keen on overriding the widths of the columns which are set by an inline style from Views by force. But 25% seems to be the only value Views provides as an inline style for grids, so it might be possible that will never change? For any amount less than 4 items the teasers just stack.

Also one more design improvement I missed.

The original grid styling had a border at the very bottom row, we should keep that in the way we use the borders now.
 

 
Please remove this code from the patch:

 .views-view-grid.horizontal .views-row:last-child {
    border-bottom: 0;
    margin-bottom: 0;
    padding-bottom: 0;
  }
pakmanlh’s picture

FileSize
320.2 KB

Updating with some thoughts. After a review of the grid views options I think we might assume that the user will uncheck the "automatic width" option to use the Bartik (or theme) configuration styles because the advice message is clear enough.

Views Grid Screenshot

After that we could prepare a finit number of col-n (col1, col-2, ...) styles, until 16 or 24 for example, and then use also the cols-n selector to determine how many columns have the grid and apply the respective percentages.
What do you think? :)

emma.maria’s picture

Issue tags: -Novice
dshesq’s picture

After trying to get a 200x200 image to display using Views > Grid, the result only produces a single column. Using the same set up in 7 gets a display of 4 images across. Before I start to look at the code, any suggestions?

lauriii’s picture

Version: 8.0.x-dev » 8.1.x-dev
Sabreena’s picture

Assigned: Unassigned » Sabreena
Sabreena’s picture

Assigned: Sabreena » Unassigned

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

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.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.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now 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.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.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.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

komalk’s picture

Re-rolled the patch against the 9.1.x

komalk’s picture

Status: Needs work » Needs review
FileSize
494.87 KB
527.71 KB
1.8 KB
2 KB

Worked on the CSS-lint issue and https://www.drupal.org/project/drupal/issues/2031447#comment-9836693.
Attached screenshot for the references.

Ruchi Joshi’s picture

@komalkolekar- On applying patch, its working fine for "Horizontal" grid view but not working for both Vertical view and Mobile version. Screenshots are attached.

Santosh_Verma’s picture

I have worked on this issue, now its working fine for Horizontal and Vertical view as well for Mobile and I pad.
I am attaching the screenshot for reference.

Ruchi Joshi’s picture

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

Moving it to "Needs work" for patch re-roll

komalk’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.66 KB

patch #77 Applied successfully.
Work on the CSS lint issues.

komalk’s picture

FileSize
1.77 KB

Forget to add the interdiff.

Ruchi Joshi’s picture

Tested issue with patch#79. New Grid view is working fine for Bartik theme. Screenshots are attached. @markcarver +1 for RTBC.

Steps:
1. Visit /admin/structure/views
2. Click on "Add view" button
3. Create a view page filling- view name and click on create a page.
4. Then click on "Save and edit" button
5. Create page with same url alias which created during creation of view.
6. Click on "Format | Unformatted list" link and click on Grid radio button.
7. Also click on "Format | Settings" link and switch between "Alignment" as Horizontal or Vertical for different views of same page.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

tanubansal’s picture

#79 works for me at 9.1
RTBC + 1

anmolgoyal74’s picture

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

Working for 9.2.x as well. Marking it as RTBC.

justafish made their first commit to this issue’s fork.

alexpott’s picture

Crediting people who provided patches and rerolls and both before and after screenshots. Thanks to everyone for all the work on moving this issue forward over the years.

lauriii’s picture

I manually tested both vertical and horizontal grids quite extensively and everything seems to work as expected. The code looks very good. Here's interdiff for few minor changes that we should still make.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed e543925 and pushed to 9.2.x. Thanks!

  • alexpott committed e543925 on 9.2.x
    Issue #2031447 by DickJohnson, komalk, markhalliwell, stephenrodrigo@...

Status: Fixed » Closed (fixed)

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