[WIP] Adding options to ColumnComponent#setWidth

[rathboma]

This adds the ability to override force when calling setWidth(true)

Why?

setting force to false allows Tabulator to take maxInitialWidth into account, like when it lays out a column for the first time.

Usage

// resize to fit contents, up to initial layout limits
column.setWidth(true, { force: false });

[olifolkerd]

Hey @rathboma

Happy to have this as additional functionality, in-line with how a lot of the other functions work, could we change this so that you just pass another value instead of true into the width property.

A single word string of some kind that isnt a valid width would be easy to pick out from the value. just a bit simpler than adding a whole config object for a simple functionality tweak.

Any thoughts on the word?

Cheers

Oli :)

[rathboma]

Hey, that sounds good. Maybe something like setWidth('initial')? So it's clear it's setting to the width as it would initially be on first render?

[olifolkerd]

Perfect

[rathboma]

So looks like this is harder than I expected.

So without persistence, initial works just fine.

Persistence causes some problems:

See Column.js#fitToData: it checks to see if widthInitiallyFixed is true (column.definition contains width). Usually this means width was provided in the column definition, but with persistence this is always true when restoring a width from the persistence store.

So in that case, there's no way to restore to initial because Tabulator thinks the value restored from persistence is the initial value. Does that make sense?

[olifolkerd]

Hey @rathboma

I think this is actually becoming two different bodies of work, one to allow restoring column values to their initial width, and another that then updates the persistence module to add the functionality you need, as from your description i would consider the existing functionality of the persistence module correct.

Looking at the content of the PR, i have a couple of points of feedback:

widthFixed

You seem to have made changes to a couple of places where it is checking to see if the widthFixed property has been set, doing this will break several other bits of logic that are dependent on knowing whether a width has been set on the column. This is to do with managing resizing of columns to fit after a user has manually resized a column and not to do with the original fixed width. so please don't change this.

If you need to reset the with and allow the width to change, then your functionality should clear the fixedWidth flag rather than change the logic that is used for other usage cases (maybe a public function to clear the flag would be helpful, covered later...).

persistence

Im a bit confused as to why all the extra logic has been added in the setColumnLayout persistence module, it should have no awareness of the values passed to the setWidth function (the initial or true), it is triggered on the value change and then reads the values from the columns themselves. when you call the setWidth function and pass in value such as true or initial that should never be stored on the column, it should calculate the correct width value and then set that on the column. Otherwise you are tightly coupling the modules and it becomes hard to manage them in future.

As for the persistence modules behaviour, i would consider the described behaviour correct as it is the initial width of the table on load. and while i can see in your usage case that isn't correct, when this is being used on a website i can see that as being the predominant way of managing things. What we may need to to add an extra callback to the persistance module that lets you intercept the persisted data before it is loaded into the columns so that you can add any additional custom functionality

If you need to have some custom behaviour at the point, i would suggest that you add a callback to the setColumnCallback function of the persistence module, that lets you intercept the column data before it is loaded into the table and make any changes that you need to it there for your custom usage case.

If you need to be able to reset the "initial" state of the column after this then can i suggest a function on the column component to help with this.

I know this sounds like a few extra steps, but it will make for an update that is more rounded and can be applied to a lot more different usage cases and in a much more flexible way.

Cheers

Oli :)

[rathboma]

Hey Oli,

So the code in this PR wasn't ever really designed to be merged, I mostly just ended up exploring the problems and seeing if I could 'hack' a fix in.

Here's what I think I need to do, let me know if this works:

PR 1 - setWidth('initial')

• Simply adds this functionality to the setWidth function. Ignoring any conflicts with the persistence module

PR 2 - Some way to discard persistence widths, so I can call setWidth again

• How would you suggest I do this?

[rathboma]

@olifolkerd here's another idea that solves both problems:

Instead of all that, what about this change to setWidth:

setWidth({ maxWidth: 123 })

So instead of passing int/bool, also accept an object with a maxWidth.

This does EXACTLY the same as setWidth(true), but instead of using the built-in maxWidth, it uses the one supplied.

This allows a user (me) to pass the same value I previously used for maxInitialWidth.

Thoughts? This seems simpler

[olifolkerd]

Hey @rathboma

Good idea, that does sound like a simpler proposition.

Im not keen on passing in the params object, it is not a standard part of the Tabulator API, pretty much all functions take in simple values, and i want to try and keep the API as consistent as possible.

How about we update the setWidth function, so if you set it to true, you can constrain the width, min and max through three args, setWdith(true, min, max) that would keep the existing API consistent, keep with the simple props paradigm used else where and make for a much more configurable function call that would make it a very flexible function?

Sound like a good compromise?

Cheers

Oli :)

[rathboma]

Perfect