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

[Proposal]: Add dynamic slots for KTable columns #743

Open
AlexVelezLl opened this issue Aug 21, 2024 · 15 comments
Open

[Proposal]: Add dynamic slots for KTable columns #743

AlexVelezLl opened this issue Aug 21, 2024 · 15 comments
Labels
Component: KTable TODO: needs decisions Further discussion and planning necessary type: proposal New feature or request

Comments

@AlexVelezLl
Copy link
Member

Product

KDS.

Desired behavior

I would like to be able to customize the contents of certain columns in a table using dynamic slots. For example:

We can pass an id to our headers array:

headers: [
  { label: 'Name', dataType: 'string' },
  { label: 'Age', dataType: 'numeric' },
  { label: 'City', dataType: 'string', id: "city" },
],

And for the columns that have that id we can add a slot with that name to have specific content for that column:

<KTable
  :headers="headers"
  :rows="rows"
  caption=""
  :sortable="false"
>
  <template #cell="{ content }">
    <span> {{ content }} </span>
  </template>
  <template #city="{ content }">
    <span>{{ content }} (City)</span>
  </template>
</KTable>

So if there is a column that has a dynamic slot, use that slot to render its content, otherwise use the default #cell.

Current behavior

Currently if I wanted custom content for the city column, I have to use the colIndex of that column:

<KTable
  :headers="headers"
  :rows="rows"
  caption=""
  :sortable="false"
>
  <template #cell="{ content, rowIndex, colIndex }">
    <span v-if="colIndex === 2">{ content } (City)</span>
    <span v-else>{ content }</span>
  </template>
</KTable>

The Value Add

This improves maintainability as if we change the position of the headers, or prepends any column, we wouldnt have to change the template or have to deal with counting which index corresponds to each attribute.

Possible Tradeoffs

We must find an effective way to communicate this in the api docs. Since it can be a bit confusing to have something like a dynamic slot in the slots table in the documentation.

@AlexVelezLl AlexVelezLl changed the title [Proposal]: Add dynamic slots for KTable cells [Proposal]: Add dynamic slots for KTable columns Aug 21, 2024
@MisRob MisRob added the TODO: needs decisions Further discussion and planning necessary label Aug 22, 2024
@MisRob
Copy link
Member

MisRob commented Aug 22, 2024

Nice proposal.

I'd even go as far as to make id required on all header items, rather than just adding it to the ones we want to use their corresponding named slot for. Otherwise I already see devs debugging why a named slot doesn't work, just to find out there's forgotten id :)

@MisRob
Copy link
Member

MisRob commented Oct 4, 2024

Regarding id, note that #794 will introduced that as columnId

@MisRob
Copy link
Member

MisRob commented Oct 4, 2024

@BabyElias any thoughts on @AlexVelezLl's proposal before we open a final issue out of it? Please let us know! I am also going to invite the core team for discussion now.

@BabyElias
Copy link
Contributor

Alex's proposal seems all good to me, nothing to add from my end.

@bjester
Copy link
Member

bjester commented Oct 4, 2024

When a component evolves to the point where you're passing related data via two or more pathways, I think it's worthwhile to think how you might consolidate them. For instance, the table headers metadata are essentially the props for whatever template will be used to render each column header. I know you're not proposing this specifically for the headers, but rather the table cells. Although, I do think it can apply similarly to the table cells, but that has reliance the table headers (ordering).

The main advantage I see is that you could consolidate two pieces of the rendering to make it more plainly evident without as much need for in depth explanation, which you acknowledge will be a challenge. It doesn't matter what order you define the slots as children being passed to a component-- the output of course is determined by how the component uses the slots. In the case of the table, the order of the headers is determine by related data being passed to the headers prop. So those two aspects of the rendering are disconnected from what would normally be the case for an HTML structure. If you align the headers, you could then potentially align the templating for a cell, too.

@MisRob
Copy link
Member

MisRob commented Oct 15, 2024

Thank you @bjester, would you have a moment for a rough pseudo-code example or two that would demonstrate the consolidation you describe? Overall I think what you're describing makes sense, but I don't understand yet how that'd translate into the table interface.

Generally, during the first stages of KTable development during GSOC, we focused more on its a11y aspects rather than fine-tuning public API, and we don't yet use it widely, so it's all pretty open even for breaking public interface changes and even outside the scope of this issue.

@bjester
Copy link
Member

bjester commented Oct 15, 2024

I can see many different pathways to achieve the consolidation, which all could look somewhat different. Here are some considerations that come to my mind:

  1. We can treat Vue components like objects-- thinking about the HTML API and how different element types may have different properties or methods, we can incorporate the same ideas with respect to components and treat them as entities in regards to their interactions with other components. For example, the HTMLVideoElement inherits properties and methods from HTMLMediaElement, such as the paused property and the play() method. This gives an API with which we can define particular interfaces, and inevitably helps us write stricter code because we would then enforce those APIs in our code. In the case of a table, there are likely common properties and behaviors between a header and a cell.

  2. We can encapsulate information in Vue components-- by treating components like objects, we're able to rely on encapsulated information represented in component instances. This will allow us to write code that is more aligned with the Single Responsibility Principle, since we would breakdown properties and behaviors into individual components, and then focus on the necessary APIs that are needed for composing them together into a table. This would also reduce the monolithic nature of KTable as it grows.

One idea is to take inspiration from the native <table>, for example it has many different types of elements (fits with the idea of having separate components) that can be used to build a table, but it also makes decisions, such as implementing default behaviors, depending on the elements it was given. For example, the following and how it would eventually be rendered:

<table>
  <tr>
    <th>Header 1</th>
    <th>Header 2</th>
  </tr>
  <tr>
    <td>Cell 1</td>
    <td>Cell 2</td>
  </tr>
</table>

would be rendered as:

<table>
  <thead>
    <tr>
      <th>Header 1</th>
      <th>Header 2</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>Cell 1</td>
      <td>Cell 2</td>
    </tr>
  </tbody>
</table>

The above behavior is possible because of the encapsulation of information into each type of element. So with <th> being a header cell, it determines that's a header row, and renders it as such.

My impression is that the KTable component could perhaps be better served by KTableHead and KTableBody components. Those could just be slots (avoiding slots and relying on the type of component can flatten the structure), but either way, the children of those could represent each header cell or data cell as individual components and align the ordering with additional information and functionality for each header or cell. Flexibility could be maintained by enforcing either the :headers prop is passed or KTableHead / slot is used, but not both.

<KTable :rows="rows" :sortable="true">
  <KTableHead #head>
    <KTableHeader :name="name">Name</KTableHeader>
    <KTableHeader :name="age">Age</KTableHeader>
    <KTableHeader :name="city" :sortable="false" :colspan="2">City & State</KTableHeader>
  </KTableHead>
  <KTableBody #body>
    <KTableStringCell />
    <KTableNumericCell />
    <KTableStringCell>
      <template #cell="{ content, row }">
        {{ content || row.city }} (city)
      </template>
    </KTableStringCell>
    <KTableStringCell :key="state" />
  </KTableBody>
</KTable>

The above incorporates a number of possibilities that I'm not necessarily proposing, like the idea of colspan, which could be challenging with the current proposal, but as an example showing how additional information can be composed into individual components. An alternative could be some KTableColumn which can handle both the header and cell, which has its merits, but could be more challenging to implement.

@MisRob
Copy link
Member

MisRob commented Oct 16, 2024

Thanks for your time putting your thoughts here, @bjester, this is really valuable. I'd like to invite @BabyElias and @AlexVelezLl to have a look and provide your input if you can think of anything.

@bjester In the last summary example you provide, I appreciate the way this resembles the native HTML table since it's intuitive way of working with tables. I consider breaking the logic to more components helpful and was something I already thought a bit about too. Particularly because of the future use-cases, where we will need editable cells and also possibly vertical headers. Then we could:

<KTable>
   <KTableVerticalHead #head>
      ...
  </KTableVerticalHead>

  <KTableBody #body>
    <KTableCell editable />
  </KTableBody>
</KTable>

As @BabyElias mentioned in another issue proposal if I recall, with adding new functionality, the current initial configuration objects will be growing larger and larger and this strategy would help with that. I was also reminded of a former proposal by @indirectlylit that seems to have some similar aspects. I am planning to look into it later on as we're closer to this stage of work.

One thing to clarify later would how many components we'd want to have exactly, and a discussion around the value of them compared to slots and other alternatives. I am too not yet completely clear on this but pretty sure that breaking it somehow similarly as suggested would help.

I am going to open some discussion on Slack soon regarding a roadmap of the next steps focused on KTable's public API and other use-cases, where I will reference this discussion.

Regarding the original @AlexVelezLl's proposal, I'd say it'd be best to leave in the proposal stage at this point and gather feedback like in this discussion, and then make some decisions based on our decisions around the future direction for KTable. Would that make sense, @AlexVelezLl?

@AlexVelezLl
Copy link
Member Author

Regarding the original @AlexVelezLl's proposal, I'd say it'd be best to leave in the proposal stage at this point and gather feedback like in this discussion, and then make some decisions based on our decisions around the future direction for KTable. Would that make sense, @AlexVelezLl?

Definetely! Makes sense to me! My initial proposal was based on the current KTable implementation, but didnt know that the base public api of KTable had not been fully decided yet 😅.

@MisRob
Copy link
Member

MisRob commented Oct 16, 2024

No problem, @AlexVelezLl, it'd be also possible we will have two parallel lines of work, one for the current version, and another focused on another iteration more high-level. Depends on priorities, it's all quite open at this point. In any case, I see much value in writing and discussing these proposals, because it gives us much concrete ideas and things to talk about that will later become well-crafted components :)

@AlexVelezLl
Copy link
Member Author

Hi! Coming back to this, from my perspective, I see a lot of value in keeping these components separate as Blaine proposed, because of Blaine's explanation, and because otherwise the KTable configuration objects would become increasingly complex and less scalable.

I was thinking about how we would handle sorting with this structure, I know there are several ways, using composables externally to these components that the KTable user uses, using composables internally in the subcomponents and communicating them via provide/inject (like the pattern we used for the KCard/KCardGrid), or another option is to use slots props to handle the passing of data and event handlers and have the KTable user again be in charge of making that connection.

Of these, in my opinion, the one that leaves more abstraction and ease of use for the KTable user is to use composables with provide/inject internally, since it abstracts and hides all that logic of what is the source data, what is the sorted array that is going to be displayed and who is the component that calls that sorting. In this way, the KTable would provide the array of sorted data (which would be injected by KTableBody) and the sorting methods (which would be injected by KTableHead or KTableHeader). And the final API that the user would use would be just as simple as Blaine's example.

To handle whether we are using local or external sorting, we could consider that if we send a @sort listener to the KTable it is because it will be sorted externally, and the sorted array would be the same one we send as a prop to the KTable. And if we do not send the @sort listener we will use the internal sorting as I have already described.

Although I think we can reconsider the use of such specific components as <KTableNumericCell /> and <KTableStringCell /> since the only difference between them is the alignment, so perhaps a <KTableCell type="numeric"> could be more suitable? And with this, we expose fewer components (I dont have a strong preference for this, though).

@AlexVelezLl
Copy link
Member Author

AlexVelezLl commented Nov 8, 2024

On a separate comment, this would introduce a breaking change in the KTable structure. If we decide to go with this change, would it be ok to introduce this new API in KDS v6? Which would be more aligned with a next Kolibri feature flag release. Or would we want to introduce this new API in 0.18 and also wait for this change before wrapping up KDS v5.0.0? @MisRob

@MisRob
Copy link
Member

MisRob commented Nov 8, 2024

Thank you @AlexVelezLl , I will read your comment. For now just briefly regarding release schedule, I am opening this is a longer-term discussion in preparation for future work that is not scoped yet so there is no need to be worried about the releases at all :) I could imagine that this exchange could serve well as a basis for another gsoc project that'd be about writing the detailed specification and implementing it. Or it'd serve us internally for having some initial direction should we find ourselves in need of it sooner. With a benefit that it can inform decisions around smaller features, for example like this original proposal. It's part of my attempts to have pro-active discussions around KDS APIs, in advance.

@MisRob
Copy link
Member

MisRob commented Nov 8, 2024

And when the time comes, we will connect and plan around breaking apis together for sure @AlexVelezLl

@MisRob
Copy link
Member

MisRob commented Nov 11, 2024

Hi again @AlexVelezLl,

Of these, in my opinion, the one that leaves more abstraction and ease of use for the KTable user is to use composables with provide/inject internally, since it abstracts and hides all that logic of what is the source data, what is the sorted array that is going to be displayed and who is the component that calls that sorting. In this way, the KTable would provide the array of sorted data (which would be injected by KTableBody) and the sorting methods (which would be injected by KTableHead or KTableHeader). And the final API that the user would use would be just as simple as Blaine's example.

Yes, I think ensuring that we handle internally things that don't need to be known externally will help to simplify, and this sounds like a useful pattern.

To handle whether we are using local or external sorting, we could consider that if we send a @sort listener to the KTable it is because it will be sorted externally, and the sorted array would be the same one we send as a prop to the KTable. And if we do not send the @sort listener we will use the internal sorting as I have already described.

I appreciate this idea. It would create a simpler interface as we could remove disableDefaultSorting. Also sounds intuitive.

Although I think we can reconsider the use of such specific components as and since the only difference between them is the alignment, so perhaps a could be more suitable

I too was wondering about this and too am not very opinionated. May be nice to hear from other developers and re-fine when the time comes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: KTable TODO: needs decisions Further discussion and planning necessary type: proposal New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants