My edited version

[mationai]

Just want to give back and share my edited version I'm using locally and point out some highlights and why's of the changes.

https://gist.github.com/fuzzthink/2b39d046ea1efca393623fc8c0af5687

Highlights of changes:

1. Use as single config object with key-vals of columns, rows, and pretty much all other configs. I've only added multisort because I want to override it to false.

• The main benefit is the source code is now rowsCfg.forEach((cfg) =>, {#each config.columns as cfg, iCol}, cfg.something, etc. This makes reading the code much easier as it takes no brain power to see it is a config, whereas column.something and row.something does -- it takes that extra brain power to recall it's config, and that's not a good thing. Yes, the code can be changed to this w/o changing the interface, but why not introduce this benefit to the caller too?
• The counterpart to it is the caller, it is much simpler <ExtTable data={rows} {config}>. And definition of config is clearly about config as well.

  const columns = [
    { propertyPath: 'acct', title: 'Acct', sortable: true },
   ...
  ]
  const config = {
    columns,
    rows: [...],
    ...rest,
  }

2. Take out the classNames code into its own file. I love how short your code is, but it short as it is, it can be better extracting non-related code into its own file. tableClassNames.js is just that.
3. Take advantage of js tooling and use ?. to simplify the logic. The current PR can be much easier to read with ?.. Take a look at tableClassNames.js, the logic is just 4 flat conditionals, no need for nesting ifs.
4. Repeating the same code just because one has a on:click|stopPropagation and one doesn't is code smell to me. 90% of errors happens because we forget to apply the same fix needed to all the different places. I'm probably missing the need for the stopPropagation, but until I run into that problem, I'm not going to let that mess up my code. And if I do run into it, I believe there should be better solutions.
5. Don't build something YAGNI. Supporting 20 columns of slots added 80 lines (40 lines if you take away the repeated code above). If you do need 20 cols of slots, don't make any changes (until svelte supports dynamic slot names). But don't ruin your code for something maybe one user out of 100/1000s may use it. For me, I think supporting just 1-3 cols in the start and end is enough.
6. Minor stuff, mostly dev preferences, but I like keeping var names short as long as it is more readable to me, using const over let to show intention, using if (...) instead of (...) && -- much easier to read imho, see tableClassNames.js, and using ternaries more (getOddEvenClass).

With these changes, the main file is just 200 lines and 260 if classNames logic is not extracted. More importantly is readability, which imho, has been enhanced.

Please don't take this the wrong way, I've already mentioned how much I like your library. Just want to share my thoughts so you can take some of these suggestions if you think they are sound.

[dritter]

Thanks for the feedback!

Use as single config object with key-vals of columns, rows, and pretty much all other configs. I've only added multisort because I want to override it to false.

That is indeed a good idea. I see the same advantages you pointed out. Only downside is that this is breaking backwards compatibility, so we need a new major version. I'll merge #8 first, push a new version and work on a 2.0 version.

Take out the classNames code into its own file. I love how short your code is, but it short as it is, it can be better extracting non-related code into its own file. tableClassNames.js is just that.

Also a good point. I'll do that in #8

Take advantage of js tooling and use ?. to simplify the logic. The current PR can be much easier to read with ?.. Take a look at tableClassNames.js, the logic is just 4 flat conditionals, no need for nesting ifs.

👍 -> #8
Btw. did you see that I simplified the row classes a bit? We just need here a plain array of objects. No need for the double-nesting. So instead of

let rowDefinition = {
        classNames: [
            {className: {propertyPath: 'location.postcode'}},
            {className: {value: (data, row, rowIndex) => (rowIndex + 1) % 2 === 0 ? data.last_name : ""}},
            {className: {value: (data, row, rowIndex) => data.title === "mr" ? "male my-other-class" : ""}},
        ],
};

we can do

let rowDefinition = {
        classNames: [
            'my-row-class',
            {propertyPath: 'location.postcode'},
            {value: (data, rowIndex) => (rowIndex + 1) % 2 === 0 ? data.last_name : ""},
            {value: (data, rowIndex) => data.title === "mr" ? "male my-other-class" : ""},
        ],
    };

epeating the same code just because one has a on:click|stopPropagation and one doesn't is code smell to me. 90% of errors happens because we forget to apply the same fix needed to all the different places. I'm probably missing the need for the stopPropagation, but until I run into that problem, I'm not going to let that mess up my code. And if I do run into it, I believe there should be better solutions.

Yes. This is a hack. Not sure if it wasn't possible in the past, or if there is a better was nowadays. I'll address this in a separate issue.

Don't build something YAGNI. Supporting 20 columns of slots added 80 lines (40 lines if you take away the repeated code above). If you do need 20 cols of slots, don't make any changes (until svelte supports dynamic slot names). But don't ruin your code for something maybe one user out of 100/1000s may use it. For me, I think supporting just 1-3 cols in the start and end is enough.

Actually, I have around 12 columns in my side project (some heavy tables there) where I extracted this library from. So the step to 20 columns was not that far.

Minor stuff, mostly dev preferences, but I like keeping var names short as long as it is more readable to me, using const over let to show intention, using if (...) instead of (...) && -- much easier to read imho, see tableClassNames.js, and using ternaries more (getOddEvenClass).

👍 -> #8

Please don't take this the wrong way, I've already mentioned how much I like your library. Just want to share my thoughts so you can take some of these suggestions if you think they are sound.

No worries. I appreciate your feedback 😃

[mationai]

Nice change on row def structure. Glad it helps.

[mationai]

Closing it, as it can still be referenced.