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

Make the table a grid css property based table #41

Closed
Ennoriel opened this issue Sep 11, 2021 · 9 comments
Closed

Make the table a grid css property based table #41

Ennoriel opened this issue Sep 11, 2021 · 9 comments

Comments

@Ennoriel
Copy link
Contributor

This library is awesome. 🌟 I have used it in two projects so far.

My first need is to have an explanatory row that shows below a row when the row itself is clicked is clicked. I tried to achieve that by various way in defining a second <tr/> in the Row slot. It works but then it's quite difficult to style the <tr/> element to have a nice in/out effect.

I'd like to suggest an evolution (I could make a PR if it makes sense and according to the best solution) : making the table a grid based table

There are many options to achieve that but none of them fills the right way to do it:

  • a property isGrid: it comes with drawbacks: the header slot would not have a sense anymore.
  • a table, tbody and thead slots: I did not achieved to successfully insert a slot in a slot but it might be possible.
  • a table, tbody and thead components / rendering methods: it will work and like the previous option, it will allow anyone to not display a table, tbody and thead tag but it fills quite complicated
@dasDaniel
Copy link
Owner

Thanks for the feedback.

My goal with this package/library was to create a table solution that makes sorting, filtering, and searching data, both easy and flexible; it imposes very minimal styling and can easily be styled by passing some classes.

However the implementation, as-is, has some trade offs or shortcomings. For example, there is no pagination implemented, and no interface to tie a pagination component into it. This means that vertical scrolling is required for large datasets, where the header is not going to stay (unless some css is implemented by user). Horizontal scrolling is also complicated and requires custom css that renders table components as inline blocks. Both of these issues seem, to me anyway, to be rooted in the use of tables.

The issue I'm struggling with making additional changes is that I don't want to introduce breaking changes, and I don't want to complicate the code.

This project started as me sharing a test porting a similar implementation I created in Vue. This is largely why the repo is in such a state currently (specifically the single .svelte file with all the filter, search and render logic in there and no automated tests). However, having snagged a snappy name, (svelte-table) and having some minimal demo has gotten me some users now (over 1000 weekly downloads via npm) and I don't want to start breaking anyone's implementation.

On the other hand, if I keep all the existing implementation as is, and add features such as conditional rendering of divs with grid instead of tables, that can increase the code by 30%-40%, which in itself is not a problem, but makes the code more complicated and harder to maintain. Some of the feedback I've received indicated that some people just copy-paste the implementation and make the changes needed for themselves; increasing complexity makes the code less readable in those cases.

If I had to chose between the two, In the short term, not breaking anything is more important that the code quality of the project,
However, compounding the tech debt is also a problem and if there is a breaking change needed it is usually better to introduce it sooner so fewer users are impacted. I had thought of "refactoring" this project with some major (breaking) changes. Given that it's a 0.1.* version a change to 1.*.* with breaking changes may be excused🤷‍♂️.

I've been thinking for a while of implementing with a more modular, component-based approach.

instead of a config object based configuration like:

<script>
  import SvelteTable from "svelte-table";
  const rows = [
    /** ... */
  ];
  const columns = [
    /** ... */
  ];
</script>

<SvelteTable columns="{columns}" rows="{rows}"></SvelteTable>

It would look more like this component-based setup:

<script>
  import { Table, Filter, Search, ...etc } from "svelte-table";
  const rows = [...];
</script>

<table>
  <head>
    <HeadRow>
      <HeadCell>Name <Search></Search></HeadCell>
      <HeadCell key="age">Age<Filter filterFn="{filterAge}"></Filter></HeadCell>
    </HeadRow>
  </head>
  {#each items as row}
  <Row>
    <Cell>{row.name}</Cell>
    <Cell>{row.age}</Cell>
  </Row>
  {/each}
</table>

This is similar to another project that already uses this svelte-accessible-table. I've done something similar with svelte-listbox, and although it is nice to be able to control things this way it seems to make a simple implementation more complicated. Another implementation, more similar to the current implementation is svelte-tabular-table. Neighter of the two offer the filtering and searching but svelte-tabular-table has sorting and some nice features such as rearranging rows and autohide. These and several other table options are now available and I'm ot clear on where this project fits in a larger eco-system with more choices.

TL;DR;

In short, I'd like to do an refactor/update for this library, but need to figure out the ballance between ease and versatility. In the meantime, if you could share an example repl of the project you're facing, maybe this can be addressed even with some css that won't need changes to the functionality.

@Ennoriel
Copy link
Contributor Author

The new trend in React is headless UI libraries (like the v.7.x.x of react-table which is what you are describing. The library provides APIs, the developer build the UI.

It fills much more complicated to learn, with a lot of boilerplate code to write (keyboard, mouse, touch event handling, etc.), especially in a js world where you can easily handle 50 libraries in a project. It is also not usable as is for a simple MVP where you are searching a library like svelte-table today.

In theory, the headless lib could be framework agnostic. In theory only, because it usually use framework dependant mechanisms (data storing, update cycle, typescript types...).

One good alternative would be to make a headless UI lib and provide a UI lib implementing it.

As another example, I have tried to make my first library ever: a color picker. There are two complexities: gradients & event handlers which makes it complex to not bundle the UI with the lib. When I made it I did not thought about headless lib. I tried to make it as much customizable as possible with a lot of components that can be passed as props.

Whatever path you take, I'd be glad to help. ;)

@dasDaniel
Copy link
Owner

dasDaniel commented Sep 13, 2021

The solution I'm leaning toward is something like the headless ui library, with a bunch of the complexity being modular. This would, ideally, make it support user-defined functionality.

I've also played with this when creating svelte-listbox repl. But the amount of boilerplate for even the simplest implementation could only appeal to a react dev 🤣.

To remedy that, I think the library would come with a default table SvelteTable, that would behave (almost🤷‍♂️) the same way as the current version.

ON a side note, I like how vuetify(I think) has a table where you can use a slot name to define a key. I really wanted to do something similar, but the svelte slot names cannot be dynamic, so it seems that that would be impossible.

I think for now I'll try some repl tests to see if I can model the kind functionality I described (headless with default component). Then move on to creating a branch with typescipt, tests etc...

help would be appreciated

@dasDaniel
Copy link
Owner

I've been trying some different things out, repl

I'm running into some potential issues with svelte that I think is a bug (issue #6734), but even if it gets resolved, I'd like to have compatibility with older versions, so I'll need another solution.

@trasherdk
Copy link

That STTable thingy looks very promising 👍

@dasDaniel
Copy link
Owner

I keep hitting a wall here and may abandon this headless component-based refactor.

The problem is that svelte is just react, and some options are just not available and the workarounds seem a bit convoluted.

One of the important features of this table component is that it does row filtering and ordering. I'm trying to have a top level headless component handle these things and allow these other components to integrate with it.

The slot can make a value from the component available, but only within the slot content template
image

using the bind:this doesn't work well for accessing the variables, because the needs additional logic to wait for the context to be updated, and passing writables is also causing issues because the values can't use the $ magic.

svelte-simple-datatables deals with this by using a store singleton for handling the data. This would prevent someone from using two table on the same page.

Then there's this bug that I keep seeing (it, doesn't break anything but the errors are not good)
image

These can all be overcome, with some special handling, but I'm getting to the point where I just don't see this working effectively. I'm not at feature parity yet, and the new SvelteTable component and It's already more complex and has more code. The idea that the internals would be exposed and available to compose a variant with custom functionality just seems a bit out of reach. The API to allow that would be too complex and/or require understanding of inner workings. The grid table with limited functionality from the repl with no filtering or searching is pretty straight forward and can have features like expand row added easily.

I think for the time being, I'll just do some code cleanup and add ability to expand rows (by adding another row below)

As far as using css grid instead of table, you can assign these styles to table elements (example)

@dasDaniel
Copy link
Owner

I have a draft PR open for expanding rows ST-047.

@Ennoriel
Copy link
Contributor Author

Ennoriel commented Nov 2, 2021

Sorry for not being able to reply earlier.

I just read the merged PR and tried it out. It works like a charm!
Thanks a lot!

@dasDaniel
Copy link
Owner

@Ennoriel just a heads up that with #63 the v0.3.0 release, there is a breaking change around the naming of expanded row class.

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

3 participants