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

Fit columns without forcefitcolumns=true #603

Closed
jr01 opened this issue Apr 29, 2021 · 19 comments
Closed

Fit columns without forcefitcolumns=true #603

jr01 opened this issue Apr 29, 2021 · 19 comments

Comments

@jr01
Copy link

jr01 commented Apr 29, 2021

When resizing a column and forceFitColumns=true all columns always stick to the viewport 100%. The right most column's right border sticks to the viewport right border and a horizontal scrollbar is never shown. When resizing a single column the next column grows/shrinks until it's max/minwidth and then the next column etc.

forceFitColumns is deprecated and we get a console.log message now: forceFitColumns option is deprecated - use autosizeColsMode.

Having read https://github.com/6pac/SlickGrid/wiki/Auto-Column-Sizing my expectation is that I could get a similar sticky fitting behavior with autosizeColsMode=FitColsToViewport and all columns set to autosizeMode=GUI and sizeToRemaining=true.

But when I resize a column with those options and also with any other combo of settings I tried, the right border of the right column never sticks to the right border of the viewport: either a horizontal scrollbar appears, or there is margin.

I tried this with http://6pac.github.io/SlickGrid/examples/example-size-to-content.html and in my application.

Perhaps I'm missing some setting. How can I get the (old) sticky/fitting behavior with autosizeColsMode?

@ghiscoding
Copy link
Collaborator

ghiscoding commented Apr 29, 2021

Hey @jr01, I believe you are using Angular-Slickgrid so I must mention couple things in this regard.
So in Angular-Slickgrid, what I'm using in the lib is the following

  1. by default the resize still uses autosizeColumns which is basically the forceFitColumns but only on the first page load
  2. even though Ben changed the default to autosizeColsMode that is not the one I use in Angular-Slickgrid, it is still autosizeColumns (number 1)
  3. if you want the resize by content then you should take a look at the new Angular-Slickgrid Example 30 - Resize by Content, I released it just a few days ago with latest version.
    • it's similar to the SlickGrid Auto-Column Sizing but with a few more settings and it also goes through the Formatters (if you have any) which I don't think the SlickGrid auto sizing does
    • I didn't have a chance to create the Wiki yet for it, you can follow the demo code

If you're really looking at just the SlickGrid Examples (not the Angular-Slickgrid examples), then you can change the autosizeColsMode grid options (you can see all options below from slick.core.js file). I think what you want is the following (that is what I use in Angular-Slickgrid)

gridOptions = {
  autosizeColsMode: Slick.GridAutosizeColsMode.LegacyForceFit, 
  // or...  autosizeColsMode: 'LFF' 
  // ...
};

SlickGrid/slick.core.js

Lines 714 to 748 in 4edcca2

"GridAutosizeColsMode": {
None: 'NOA',
LegacyOff: 'LOF',
LegacyForceFit: 'LFF',
IgnoreViewport: 'IGV',
FitColsToViewport: 'FCV',
FitViewportToCols: 'FVC'
},
"ColAutosizeMode": {
Locked: 'LCK',
Guide: 'GUI',
Content: 'CON',
ContentIntelligent: 'CTI'
},
"RowSelectionMode": {
FirstRow: 'FS1',
FirstNRows: 'FSN',
AllRows: 'ALL',
LastRow: 'LS1'
},
"ValueFilterMode": {
None: 'NONE',
DeDuplicate: 'DEDP',
GetGreatestAndSub: 'GR8T',
GetLongestTextAndSub: 'LNSB',
GetLongestText: 'LNSC'
},
"WidthEvalMode": {
CanvasTextSize: 'CANV',
HTML: 'HTML'
}

@jr01
Copy link
Author

jr01 commented Apr 29, 2021

Yes, @ghiscoding I'm using your Angular-Slickgrid. But even on your demo site, without the deprecated forcefit, there appears to be no way stick the right most column to the right edge all the time. For example with the (default resize) by "autosizeColumns" on and dragging the right border of % Complete with the mouse to the right I get a horizontal scrollbar which is undesirable.

@ghiscoding
Copy link
Collaborator

I'm not totally sure what you mean, can you do a print screen or an animated gif?

Other things to maybe consider, in Angular-Slickgrid I enable by default the alwaysShowVerticalScroll grid option, that is slight hack to get the Grid Menu (aka hamburger menu) to not overlap the last column. Basically the size of the vertical scroll is roughly the same as the Grid Menu and that is why I always show it. I spent almost an entire day on trying to find a way to fix the CSS for that but I couldn't and so I use that hack... could that be related to what you're describing? You can turn the flag but you will have overlap on the last column if you use the Grid Menu.

@jr01
Copy link
Author

jr01 commented Apr 29, 2021

I created an animated gif and put in some captions to explain, but only applied them to a single frame (doh!). So here is some text to explain what I try to show

The first part is with forceFitColumns on. Then I resize a single column. Initially increasing the columns size and then decreasing it. In any case the right most column's right edge sticks nicely to the right edge of the grid. That's what I want.

The second part is without forceFitColumns and the new option on. The initial column resizing is superb! Then I resize the same column and you can see that the right most column's right edge doesn't stick and either a horizontal scrollbar appear, or margin. Both I don't like.

Don't get me wrong here, I absolutely like the various new initial auto column sizing options and would really like to use them. It's just about resizing a single column with the mouse that I always would like to occupy exactly and no more than 100% of the container.

Animation

@ghiscoding
Copy link
Collaborator

ghiscoding commented Apr 29, 2021

The thing to note is that in Angular-Slickgrid I call autosizeColumns() (legacy method) which is exactly the same as doing a Force Fit Column but only on the initial page load (in other words autosizeColumns is equal to Force Fit Column). However in my repos, I didn't want to restrict the user to expand a column wider than the viewport (after the page is loaded), so in short what I do in Angular-Slickgrid is like setting the grid option forceFitColumns: true then calling autosizeColumns() and finally setting it back to forceFitColumns: false. If you want the force fit to be always set, then just change it in your grid options forceFitColumns: true and you'll now always have it (but I don't personally like that behavior and that is why it's not enabled by default in Angular-Slickgrid). I don't like the Force Fit because whatever column(s) you expand will have to be taken out of another column width and most user (me included) hate that behavior, it's only ever good if your grid doesn't have too many columns and you have plenty of space to play with (a large viewport).

It's just about resizing a single column with the mouse that I always would like to occupy exactly and no more than 100% of the container.

Isn't that Force Fit Column that you want? If so, and like I said in previous paragraph, just set the grid option forceFitColumns: true or in some other use case, you can simply call autosizeColumns() to do a single Force Fit (which is what I do when you unselect columns to show from Grid Menu)

The Example 30 should only be referenced if you want to use the new Resize by Content and once a width is set on a column definition (at load time or after a resize) it will not override it (once it's set, it's set). On the other hand the Force Fit (same thing for autosizeColumns()) doesn't care about the width since it will override it anyway (however, it does care about minWidth and maxWidth though).

... so all that to say, you might just want to use forceFitColumns: true and be done with it, or call autosizeColumns() in some occasions

@6pac
Copy link
Owner

6pac commented Apr 30, 2021

Thanks for taking this one up, @ghiscoding!

Just should say that I put the new autosizing feature in a while back, but I have found it still quite difficult to get good results with it on 'Auto' mode. It's not that there's anything wrong - it's just a quite hard problem. I think I need to go back and do some more tweaking.
So I'd say, don't be afraid to use the legacy mode if that works better for you.

I will be doing some more testing soon, I'll keep this issue in mind when I do.

@6pac
Copy link
Owner

6pac commented Apr 30, 2021

Just had an actual read of your post with the GIF - it looks like we have two different features. The new autosizing was not intended to keep the columns fitting in the viewport, it was just a one-off autosize. forceFitColumns does more, actually dynamically resizing. Not sure how these interact - I suspect that the new autosize simply didn't support the forceFitColumns feature.

@ghiscoding
Copy link
Collaborator

@6pac just to be clear, I built my own auto-resize by cell content (the Angular-Slickgrid Example 30) and so I'm not using yours because of many different reasons but the biggest is probably because mine will also process any Formatters defined and I also added a bunch of extra column & grid options like resizeExtraWidthPadding, resizeMaxWidthThreshold, and a bunch of other options. We are shipping the first few grids with this new resize...well actually tomorrow haha.

Also in order to be able to do my own auto-resize, I needed to expose the reRenderColumns as a public method, which if you remember, I did fairly recently in this PR #583

@6pac
Copy link
Owner

6pac commented Apr 30, 2021

@ghiscoding good to know. yes, that was my experience too, it's really hard to separate the sizing from the controls when it's not just text.

@jr01
Copy link
Author

jr01 commented Apr 30, 2021

I was just looking at https://docs.telerik.com/devtools/winforms/controls/gridview/columns/resizing-columns-programatically and they use the term 'fill' which better describes what I would like to keep.

@ghiscoding - I agree that the filling behavior is not good for many use cases. However filling is what my users are used to - it's been the only option in our Windows products for over 25 years now, so it's hard to give up on that.

I guess either a fill option or not deprecating forceFitColumns=true (and getting rid of the console.log message) is what I'm asking for.

@6pac
Copy link
Owner

6pac commented Apr 30, 2021

Entirely reasonable.
The implication of forceFitColumns is that there will be further auto-resizing of columns on resizing of one column.
I suppose for me, I'm wondering what algorithm to be using there.

For example, if you resize three columns then I suppose that you won't want any of those three to be subsequently auto-resized, so that means we have to track manually sized columns, which is something we don't do at the moment.

@ghiscoding
Copy link
Collaborator

ghiscoding commented May 6, 2021

I'm adding new onColumnsResizeDblClick event in PR #605 so that I could eventually go over a column and simply double-click on its resize that will call a resize by content of that column only (pretty much the same as Excel), @jr01 this might help users avoiding to have to resize themselves

image

hQ8mk7Gn9N

@cout
Copy link

cout commented Jun 15, 2021

We use forceFitColumns to widen the columns to remove any whitespace on the right. For whatever reason that I have not identified, using enableAutoSizeColumns causes the grid to not show up at all, regardless of the value of autosizeColsMode. We are using a custom pager control and an older release of slickgrid (2.4.9), so either one of those might have something to do with it; I have not had time to investigate.

The reason we have held back from upgrading slickgrid is not just time; 2.4.9 is the last release that did not log a message to the console about forceFitColumns being deprecated. As the saying goes, "if it ain't broke, don't fix it", so we are happy on the older release for now, but we don't want to stay there forever.

All of this is to say that I would like to echo the request from @jr01 to not deprecate forceFitColumns, at least not yet, until there are equivalent, stable options in place to replace it.

I also like the word "fill" as a description of the desired behavior. IIRC, this is also the term used by gtk (https://developer.gnome.org/gtk3/stable/GtkBox.html#GtkBox--c-fill). It is contrasted with "expand" (fill causes the child to grow to fill the parent; expand causes the child to grow as the parent grows).

@6pac
Copy link
Owner

6pac commented Jun 15, 2021

Ok, so as far as I can see, the forceFitColumns code has not actually been touched - it's still there and still works, up to the current version.
I think the easiest solution is simply for me to remove the message.

It should be possible to reproduce forceFitColumns behaviour with

 options.autosizeColsMode = Slick.GridAutosizeColsMode.LegacyForceFit;

I'll go over and check the interactions to make sure this is the case, and revisit the ForceFit behaviour in the new mode. That should take us into the future.

@ghiscoding
Copy link
Collaborator

I guess either a fill option or not deprecating forceFitColumns=true (and getting rid of the console.log message) is what I'm asking for.

Since I believe you use Angular-Slickgrid then I assume there is nothing left to do with this issue since the deprecation was removed in Slickgrid-Universal and forceFitColumns remains a valid option (without any console anymore with latest release). Also like I said, in Angular-Slickgrid, I do use autosizeColumns() on page load (which is equivalent to a single usage forceFitColumns) . If users wants to keep it for not just the page load, then the user can simply set forceFitColumns: true

So I don't think there's anything left to do here, so I'll simply close the issue. If the problem still remains then we need more info and ideally a full repro. Thanks

@jr01
Copy link
Author

jr01 commented Jan 17, 2024

Yes @ghiscoding we're on Angular-Slickgrid and good choice to close this one!

@cout
Copy link

cout commented Apr 9, 2024

I am confused why this issue was closed. The deprecation message is still in the 6pac repository (

console.log('forceFitColumns option is deprecated - use autosizeColsMode');
).

In 2021, @6pac said:

Ok, so as far as I can see, the forceFitColumns code has not actually been touched - it's still there and still works, up to the current version. I think the easiest solution is simply for me to remove the message.

From what I can tell, that is still the case; all the forceFitColumns code is still there, and it looks safe to remove the deprecation message. I have not yet tested though with the latest version.

@6pac
Copy link
Owner

6pac commented Apr 9, 2024

@cout you are correct, @ghiscoding removed it in a downstream project but it has not been removed here. I'll do so and this should flow through to production in a few weeks.

6pac added a commit that referenced this issue Apr 9, 2024
ghiscoding pushed a commit that referenced this issue Apr 11, 2024
* Remove ForceFit deprecation message fixes #603

* Remove message completely
@ghiscoding
Copy link
Collaborator

ghiscoding commented Apr 16, 2024

@cout the console warning was completely removed in latest v5.9.1

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

No branches or pull requests

4 participants