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

Fixes and improvements in configureColumnWidths #508

Closed
wants to merge 3 commits into from
Closed

Fixes and improvements in configureColumnWidths #508

wants to merge 3 commits into from

Conversation

ebbe-brandstrup
Copy link

configureColumnWidths

  • Columns sized with * or % now expand / shrink as other * or %-based columns are hidden / shown
    • Note: the changes auto-expand/shrink only take effect on-the-fly if using the ng-grid-layout plugin
    • Works with grouping and when enabling the selection checkbox row (showSelectionCheckbox)
  • Bugfixes in configureColumnWidths
    • Re-ordered columns now keep their width setup
    • Fixed "asteriskNum" so it no longer includes hidden columns (was checking .visible on a columnDefs column instead of the matching ngColumn)
    • Fixed "totalWidth" so it no longer includes hidden columns when using px values for width (was checking .visible on a columnDefs column instead of the matching ngColumn)
    • Fixed ngColumn width being initialized to undefined when using "auto" for width, regardless of "minWidth" settings (was checking .minWidth on a columnDefs column instead of the matching ngColumn)

Renamed "col" to "colDef" in configureColumnWidths() in the places where "col" was a column from "columnDefs". It made it clearer for me whether I was referring to a ngColumn or a column from columnDefs. There were a couple of bugs caused by that (col.visible incorrectly accessed on columnDefs objects instead of ngColumns, and the like).

ng-grid-layout plugin

  • ng-grid-layout plugin now listens to 'ngGridEventColumns' events and then rebuilds the grid

ng-grid-flexible-height plugin

  • Bugfixes in ng-grid-flexible-height
    • The plugin couldn't shrink the grid, only grow it
    • Using domUtilityService.UpdateGridLayout instead of grid.refreshDomSizes which correctly grows the grid if it's been shrunk (e.g. when paging to the last page and it has few rows + the plugin has a smaller min. height than what's needed on the other pages)

Ebbe Brandstrup added 2 commits June 25, 2013 10:34
…ly take effect on-the-fly if using the ng-grid-layout plugin)

- Columns sized with * or % now expand / shrink as other * or %-based columns are hidden / shown
- Re-ordered columns now keep their width setup
- Bugfixes in configureColumnWidths
  - Fixed "asteriskNum" so it no longer includes hidden columns (was checking .visible on a columnDefs column instead of the matching ngColumn)
  - Fixed "totalWidth" so it no longer includes hidden columns when using px values for width (was checking .visible on a columnDefs column instead of the matching ngColumn)
  - Fixed ngColumn width being initialized to undefined when using "auto" for width, regardless of "minWidth" settings (was checking .minWidth on a columnDefs column instead of the matching ngColumn)
- ng-grid-layout plugin now listens to 'ngGridEventColumns' events and then rebuilds the grid

Renamed "col" to "colDef" in configureColumnWidths() in the places where "col" was a column from "columnDefs". It made it clearer for me whether I was referring to a ngColumn or a column from columnDefs. There were a couple of bugs caused by that (col.visible incorrectly accessed on columnDefs objects instead of ngColumns, and the like).
  - The plugin couldn't shrink the grid, only grow it
  - Using domUtilityService.UpdateGridLayout instead of grid.refreshDomSizes which correctly grows the grid if it's been shrunk (e.g. when paging to the last page and it has few rows + the plugin has a smaller min. height than what's needed on the other pages)
@hlangli
Copy link

hlangli commented Jun 25, 2013

This is really great - I've been looking for this functionality sincy a long time ago. Great work!

@jonricaurte
Copy link
Contributor

@ebbe-brandstrup Thanks for the pull request. I'm going to look at/test it more when I get home today.

@ebbe-brandstrup
Copy link
Author

@jonricaurte Thanks, much appreciated.

If anything strikes you as odd or doesn't work, please let me know and I'll change things accordingly. We're depending on these changes in a project I'm working on atm. and would of course like them to be part of an official release at some point to be able to stay on the main repo.

Looking forward to your feedback.

@jonricaurte
Copy link
Contributor

I pushed a 2.0.7 branch last night that made a few changes to get rid of the subtracting for border width on columns. Instead of using borders, I'm using vertical bars so if someone sets a width to be 400px for a column, the column will actually be 400px, not 401-402px due to the border. This caused the horizontal overflowing to happen producing a horizontal scrollbar. I also moved percent calculation ahead of the asterisks calculation cause I feel percent calculation takes a higher priority, and the asterisks calculations will then be able to fill the remaining space instead of overflowing the viewport. If everything looks good on your pull request (so far it does), I'll merge your changes into the 2.0.7 branch and write some tests.

@jonricaurte
Copy link
Contributor

@jonricaurte
Copy link
Contributor

I merged your changes into the 2.0.7 branch. Can you test out your grids with the other changes in it? It can be accessed through the previous comment's link.

@ebbe-brandstrup
Copy link
Author

Great! On my way into the office now and then I'll pull the 2.0.7 branch
and test things out and get back to you.

Thanks a lot for being so quick on this - and for ng-grid in the first
place! :-)

On Wednesday, June 26, 2013, jonricaurte wrote:

I merged your changes into the 2.0.7 branch. Can you test out your grids
with the other changes in it? It can be accessed through the previous
comment's link.


Reply to this email directly or view it on GitHubhttps://github.com//pull/508#issuecomment-20027662
.

@jonricaurte
Copy link
Contributor

Yup :D I'm going to close this request and point everyone to #511

@ebbe-brandstrup
Copy link
Author

Great! I've been running with 2.0.7 for the last couple of hours and
haven't encountered any issues. The border / vertical bar fixes are nice
too and now my columns make use of all the available width (before I had a
bunch of wasted pixels to the far right).

On Wed, Jun 26, 2013 at 7:40 AM, jonricaurte [email protected]:

Yup :D I'm going to close this request and point everyone to #511#511


Reply to this email directly or view it on GitHubhttps://github.com//pull/508#issuecomment-20028108
.

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.

3 participants