Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in calculating number of columns #31

Open
surimohnot opened this issue Jul 1, 2019 · 6 comments
Open

Error in calculating number of columns #31

surimohnot opened this issue Jul 1, 2019 · 6 comments
Labels
enhancement New feature or request Good Fix Proposed

Comments

@surimohnot
Copy link

surimohnot commented Jul 1, 2019

There is a minor but substantial error in calculating number of columns. I will try to explain it below,
As per your code,
column width = item width + gutter
number of columns = parent width / column width
However, last column don't need gutter. So it should be
number of columns = ( parent width + gutter ) / column width

you may need to change other related code as well.

@e-oj
Copy link
Owner

e-oj commented Jul 1, 2019

That's actually not an issue. The gutter is on the left of each column and the first column doesn't get a gutter.

positionItems () {
    let { cols, wSpace } = this.setup();
    let maxHeight = 0;
    let colWidth = this.colWidth();

    wSpace = Math.floor(wSpace / 2);

    for (let i = 0; i < this.items.length; i++) {
      let col = this.nextCol(cols, i);
      let item = this.items[i];
      let topGutter = col.height ? this.gutter : 0;

      // Every column is moved from the left by the value of its index
      // multiplied by the width of a single column (colWidth). The trick 
      // is that the first column has an index of 0 and is not affected by
      // colWidth (0 * colWidth = 0). As a result it has no gutter. If you
      // follow the loop on paper, every other column gets a gutter to the left.
      let left = col.index * colWidth + wSpace + "px";

Is this part of a bigger concern though? Did you get any visual errors?

@e-oj
Copy link
Owner

e-oj commented Jul 1, 2019

I forgot to mention that the "extra gutter" is added to the whitespace (wSpace), which is then split equally between both sides.

setup () {
    let width = this.container.getBoundingClientRect().width;
    let colWidth = this.colWidth();
    let numCols = Math.floor(width/colWidth) || 1;
    let cols = [];

    if (this.maxColumns && numCols > this.maxColumns) {
      numCols = this.maxColumns;
    }

    for (let i = 0; i < numCols; i++) {
      cols[i] = {height: 0, index: i};
    }
    
    // Extra gutter gets added to the overall whitespace
    // which is later split in half and used to center the grid.
    let wSpace = width - numCols * colWidth + this.gutter;

    return {cols, wSpace};
  }

@surimohnot
Copy link
Author

Yes, this is actually part of a bigger concern, which makes this awesome script practically useless for me. Actually I want to create a grid where columns are having flexible width. So my calculation was as follows,

let's say Gutter = 30px;

Now in my css I have

.column { width: 100%; } @media only screen and (min-width: 640px) { .column { width: calc(50% - 15px); // 30px Gutter for two column layout. } } @media only screen and (min-width: 1080px) { .column { width: calc(33.3333% - 20px); // 60px total gutter shared by three columns. } }

However, your script showing only 1 column above 640px and only 2 columns above 1080px (which should not happen).

Hope I am able to explain correctly.

@filipjnc
Copy link

filipjnc commented Jul 11, 2019

I'm also questioning the need for this extra gutter, which is then split into half. This results into my grid having always a gap of at least half the gutter on the left and right hand-side, no matter how ideally I size my columns.

Let's take the following scenario:
container width=1230px
maxColumns=4
gutter=10
column maxWidth=400px

I would expect a perfect alignment from x=0 to x=1230. What I have instead is a gap of 5 at the left of the first column.

@stevenamoody
Copy link

I believe I have a fix for this issue, and I opened a PR. See my proposed implementation of a responsive layout test/grid-responsive.html

@jnz31
Copy link

jnz31 commented Jul 15, 2021

had a similar issue, where my elements wouldn't align properly, even though the math was correct (2 columns, 10px gutter; element width: calc(50% - 5px)). the elements where center stacked (even though center is set to false). stevenamoody's edits solved my issue.

@e-oj e-oj added the enhancement New feature or request label Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Good Fix Proposed
Projects
None yet
Development

No branches or pull requests

6 participants