Table image and url + arbitrary content discussion

[archiewood]

Description

• Possible way of adding URL and link support
• But would be good to be able to allow adding arbitrary HTML / svelte which would be more powerful than this

Implementation details

• Adds optional height and width prop to columns
  • these are needed to control the size of the images

Checklist

• For UI or styling changes, I have added a screenshot or gif showing before & after
• I have added a changeset

Result:

[changeset-bot]

⚠️ No Changeset found

Latest commit: d0ec909

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

[vercel]

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)

[archiewood]

Re arbitrary HTML support

• The closest I got was by adding a html prop.

  • Will drop in an additional commit showing this.

• 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>

• Challenge here is that the <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?

  • This means you cannot access the Column's <slot/> contents?

[archiewood]

This has quite a few downsides:

1. 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>

2. Doesn't support putting in svelte components, only vanilla HTML

[hughess]

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:

contentType Prop

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>

Content Type Components

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

[hughess]

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.

[archiewood]

Would this be:

1. Click the image and get a larger version of the image?
2. Click the image and go through to another Evidence page, or external URL?
3. Something else?

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?

[hughess]

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?

[archiewood]

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.

[hughess]

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?

[hughess]

Nevermind, I see you have a default for alt tag in your implementation already!

[mcrascal]

Moving discussion/ tracking to #570 for the meantime.