Closed archiewood closed 1 year ago
Latest commit: d0ec909ec2c331fcc7269fd6176c4725e620888c
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated |
---|---|---|---|---|
evidence-development-workspace | ✅ Ready (Inspect) | Visit Preview | 💬 Add your feedback | Jan 4, 2023 at 11:57AM (UTC) |
evidence-docs | ✅ Ready (Inspect) | Visit Preview | 💬 Add your feedback | Jan 4, 2023 at 11:57AM (UTC) |
Re arbitrary HTML support
I think the idea situation would be a <slot>
based syntax:
<DataTable
data={tableq}>
<Column>
<!-- flag is a column of tableq containing the url to an image -->
<img src={flag} height=50px/>
</Column>
</DataTable>
<Column/>
component is not imported into the <DataTable/>
component (I don't fully understand why, but I guess it's because the user may not call the column component explicitly?
<slot/>
contents?This has quite a few downsides:
It's syntax is kinda weird (though could work on that a bit)
What I'd expect to work as a user: Slot based syntax
<DataTable
data={tableq}>
<Column>
<!-- flag is a column of tableq containing the url to an image -->
<img src={flag} height=50px/>
</Column>
</DataTable>
What this gives you: Prop based syntax
<DataTable
data={tableq}>
<Column
id=flag
html={`
<img src={flag} height=50px/>
`}
/>
</DataTable>
This is cool! Making the slot work as expected would be tricky because of the way Column
works - it's basically just updating the store used by DataTable
rather than actually getting inserted into the table.
Here are a couple of ideas on syntax:
Similar to your working examples - add props for each type of content we want to support
<DataTable data={my_query}>
<Column id=country />
<Column id=flag contentType=image someImageProp=true />
<Column id=url contentType=link openInNewTab=true />
</DataTable>
We create a component for each content type we want to support within tables. These components would need to update a store that Column
could access before passing it to DataTable
<DataTable data={my_query}>
<Column id=country />
<Column id=flag>
<Image someImageProp=true />
</Column>
<Column id=url>
<Link openInNewTab=true />
</Column>
</DataTable>
If we were to go down this path, we might want to name the components more specifically so they wouldn't be confused with components that could be used outside of a table cell. E.g., CellImage
Another thought on this - should we support clickable images?
In that case, we'd need to support using img
and link
at the same time.
Would this be:
For 1, you don't need a unique prop, as it's the same URL as the img?
For 2, you could solve this if you make the whole row clickable using the link field? (though maybe that needs to be a prop - rowLink=True
)
If it's something else, maybe I'm thinking about this wrong?
I was thinking of 2 - I think a row link would solve that! I have a prototype of row links working in another branch and I'll share that in a PR soon.
So that means we'll be good with treating images and links as separate items in table. The last thing I'm trying to figure out is how these will fit into the features we plan to add in the future within table cells, like heatmaps, bar charts, progress bars, etc.
Because these new features wouldn't overlap with images and links, I'm leaning towards having a single prop contentType
to define the type within each column. For example:
<DataTable data={countries} rowLinks=country_url >
<Column id=flag contentType=image height=50px />
<Column id=country_url contentType=link linkLabel=country_name openInNewTab=false />
<Column id=sales contentType=bar />
<Column id=orders contentType=heatmap />
<Column id=yoy_growth contentType=progressBar />
</DataTable>
That feels a bit suboptimal for the link column, but convenient for the other column types.
We could also call that prop type
or something similar to make it easier to remember, but we'll also need to avoid confusion with data types.
This structure might also allow us to support html in the future (contentType=html
), but I'm not sure how that would work.
@archiewood what do you think of this syntax?
I like it. For the image we need to include the ability to add an alt tag I think. Perhaps we make it an additional optional prop?
If users don't add the alt tags then svelte will complain, at which point they can choose to add them if they want.
That makes sense to me. Is there a sensible default we should use as the alt tag, or better to leave it empty unless specified?
Nevermind, I see you have a default for alt tag in your implementation already!
Moving discussion/ tracking to #570 for the meantime.
Description
Implementation details
Checklist
Result: