conversionxl / aybolit

Lightweight web components library built with LitElement.
https://conversionxl.github.io/aybolit/
MIT License
7 stars 8 forks source link

feat: add featured course card component #294

Closed kertuilves closed 1 year ago

kertuilves commented 1 year ago

https://app.clickup.com/t/861n0qjha

pawelkmpt commented 1 year ago

Did you branch it off feat/dashboard or master? It should be from feat/dashboard. We should see here just your commit with the component.

anoblet commented 1 year ago

I believe we should be leveraging more of the benefits that custom elements provide us. Rather than defining the entire template as slotted content, I feel the template should be defined within the component and data passed in. We can be limited when the data is a list, generated html, or long content though other parts should be defined within the shadow dom. Ideally it would look like this:

<cxl-featured-course-card>
  <span slot="title">...</span>
  <span slot="tags">...</span>
  <div slot="description>...</div>
</cxl-featured-course-card>

This would allow us to scope CSS as much possible, as well as reduce the amount of data downloaded (the custom element would be responsible for rendering most of the HTML.)

freudFlintstone commented 1 year ago

I believe we should be leveraging more of the benefits that custom elements provide us. Rather than defining the entire template as slotted content, I feel the template should be defined within the component and data passed in. We can be limited when the data is a list, generated html, or long content though other parts should be defined within the shadow dom. Ideally it would look like this:

<cxl-featured-course-card>
  <span slot="title">...</span>
  <span slot="tags">...</span>
  <div slot="description>...</div>
</cxl-featured-course-card>

This would allow us to scope CSS as much possible, as well as reduce the amount of data downloaded (the custom element would be responsible for rendering most of the HTML.)

Sorry for intruding a bit here. On top of @anoblet suggestions, which I agree with, this design is very, very similar to the cxl-course theme. And the lightweight card can easily be achieved by just hiding a couple sections from the other similar components.

My suggestion would be to briefly go over the figma designs marked as ready, and come up with a revised list of components that are each related to one clickup task and one pull request.

Consolidating design variations under the same component will probably avoid a lot of headaches in the future.

pawelkmpt commented 1 year ago

I believe we should be leveraging more of the benefits that custom elements provide us. Rather than defining the entire template as slotted content, I feel the template should be defined within the component and data passed in. We can be limited when the data is a list, generated html, or long content though other parts should be defined within the shadow dom. Ideally it would look like this:

<cxl-featured-course-card>
  <span slot="title">...</span>
  <span slot="tags">...</span>
  <div slot="description>...</div>
</cxl-featured-course-card>

This would allow us to scope CSS as much possible, as well as reduce the amount of data downloaded (the custom element would be responsible for rendering most of the HTML.)

I agree

pawelkmpt commented 1 year ago

@kertuilves I just merged #289, please rebase this branch, then I'll do (final) review :)