Skip to content

grid options renamed to columns & maxRows #1053

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

Merged
merged 6 commits into from
Nov 19, 2019
Merged

grid options renamed to columns & maxRows #1053

merged 6 commits into from
Nov 19, 2019

Conversation

adumesny
Copy link
Member

@adumesny adumesny commented Nov 18, 2019

Description

  • grid options width is now columns, and height is now maxRows which match what they are.
  • Old names are still supported for now (with console warnings)
  • updated test (required), samples (optional), readme
  • re-wrote entire custom column section. Hopefully much easier to find/use now.

Created a sample for #810 with 30 columns, but not seeing issues in that PR (issue rounding cellWidth() with large # of columns). I was able to move and resize and things lined up as expected even when total width / 30 was close to .5 off
@nulen please update with info.

In the meantime I found those issues:

  • fix README talking about custom columns #
  • fix SASS code to handle >12 columns as you need to override stack-item min-width (we have 8.3333% otherwise which is 12 column!)
  • while testing I incorrectly passed 'auto' for gridItem width (it's a cellHeight on grid options instead)
    so fixed code to handle wrong strings/garbage passed for x,y,w,h as I was getting inf loop instead.
  • updated test cases

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing (yarn test)
  • Extended the README / documentation, if necessary
    also ran every demo samples, and the new 30 columns sample as well.

Alain Dumesny added 2 commits November 18, 2019 10:27
Created a sample for #810 with 30 columns, but not seeing issues in that PR.

In the meantime I found those issues:
* fix README talking about custom columns #
* fix SASS code to handle >12 columns as you need to override stack-item min-width (we have 8.3333% otherwise = 1/12)
* while testing incorrectly passed 'auto' for gridItem width (prop on grid cellHeight)
so fixed code to handle wrong strings passed for x,y,w,h
* updated test cases
@coveralls
Copy link

coveralls commented Nov 18, 2019

Coverage Status

Coverage decreased (-0.4%) to 65.052% when pulling 2f57d59 on adumesny:bugfix/810 into d86a6e9 on gridstack:develop.

* grid options `width` is now `columns`, and `height` is now `maxRows` which match what they are.
* Old names are still supported for now (with console warnings)
* updated test (required), samples (optional), readme
* re-wrote entire custom column section. Hopefully much easier to find/use now.
@adumesny adumesny changed the title many columns test case, some fixes grid options renamed to columns & maxRows Nov 18, 2019

## Grid attributes

- `data-gs-animate` - turns animation on
- `data-gs-width` - amount of columns. Setting non-default value must be supported by equivalent change in CSS, [see docs here](https://github.com/gridstack/gridstack.js#change-grid-width).
- `data-gs-height` - maximum rows amount. Default is `0` which means no maximum rows.
- `data-gs-columns` - amount of columns. Setting non-default value must be supported by equivalent change in CSS, [see docs here](https://github.com/gridstack/gridstack.js#change-grid-columns).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we care about migrating those attr ? also not clear if that and options is set which ones wins.

@adumesny adumesny merged commit a4e62e2 into gridstack:develop Nov 19, 2019
@adumesny adumesny deleted the bugfix/810 branch November 19, 2019 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants