conversionxl / aybolit

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

feat: add dashboard lightweight card #289

Closed kertuilves closed 1 year ago

kertuilves commented 1 year ago

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

github-actions[bot] commented 1 year ago

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 34.65 KB (+2.1% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.87 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 25.78 KB (+1.97% 🔺)
packages/cxl-ui/pkg/dist-web/vendor.js 125.6 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js, packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js, packages/cxl-ui/pkg/dist-web/cxl-ui.js, packages/cxl-ui/pkg/dist-web/manifest.js, packages/cxl-ui/pkg/dist-web/unresolved.js, packages/cxl-ui/pkg/dist-web/vendor.js 199.06 KB (+0.62% 🔺)
heshfekry commented 1 year ago

@kertuilves just confirming that this is the component

https://deploy-preview-289--conversionxl-aybolit.netlify.app/?path=/story/cxl-ui-cxl-card--cxl-course

heshfekry commented 1 year ago

Hey @kertuilves

Would be good to see multiple cards in the same view so we can assess how they work together? Similar to this.

Right now it seems to take full width,

image

so unclear how it would function with multiple cards

image

Additionally

  1. would be good to see the indicator. - EDIT - I missed the control. I see it but as pawel mentioned its cut. Could just be the view.
  2. A problem came up yday when we where working on roadmaps from teams and personal. We probably need to have tags on there too indicating if the course has been selected in roadmap or not.
    • I would change the label from Team > Team Roadmap and from Personal > Personal Roadmap

image

pawelkmpt commented 1 year ago

A problem came up yday when we where working on roadmaps from teams and personal. We probably need to have tags on there too indicating if the course has been selected in roadmap or not.

Use vaadin badge for tags.

kertuilves commented 1 year ago

Hey @heshfekry.

  1. I modified the Course Card canvas layout and now the icon is visible
  2. Course Card now has another with-roadmaps state
  3. I added an example under cxl-tabs-slider to see multiple Course Cards in the same view
heshfekry commented 1 year ago

Hey @heshfekry.

  1. I modified the Course Card canvas layout and now the icon is visible
  2. Course Card now has another with-roadmaps state
  3. I added an example under cxl-tabs-slider to see multiple Course Cards in the same view

Looking good. The size seems to change with different states, unclear how that plays with multiple cards in the same view.

kertuilves commented 1 year ago

@heshfekry

Looking good. The size seems to change with different states, unclear how that plays with multiple cards in the same view.

I kept the card itself really basic and didn't constrain its component width. In my opinion this should be done when adding the card to the specific layout. In our example when adding it to the slider in the accordion. And those slider styles and card widths are right now visible in the accordion PR.

heshfekry commented 1 year ago

@heshfekry

Looking good. The size seems to change with different states, unclear how that plays with multiple cards in the same view.

I kept the card itself really basic and didn't constrain its component width. In my opinion this should be done when adding the card to the specific layout. In our example when adding it to the slider in the accordion. And those slider styles and card widths are right now visible in the accordion PR.

This makes sense. Does this apply to height too? for example when you introduce the tags or the names etc take more space.

kertuilves commented 1 year ago

@heshfekry

Looking good. The size seems to change with different states, unclear how that plays with multiple cards in the same view.

I kept the card itself really basic and didn't constrain its component width. In my opinion this should be done when adding the card to the specific layout. In our example when adding it to the slider in the accordion. And those slider styles and card widths are right now visible in the accordion PR.

This makes sense. Does this apply to height too? for example when you introduce the tags or the names etc take more space.

Yes. So all the cards heights will be the same and it depends on the highest card. I kept it the same way as it is for the slider cards by default.

Screenshot 2023-07-05 at 15 19 00
heshfekry commented 1 year ago

@freudFlintstone pulling you in here because you are working on upgrading the full card.

You mentioned that this could all be the same component with an attribute. Does it make sense you take over all the card components?

will confirm with @kertuilves first best path forward

heshfekry commented 1 year ago

@freudFlintstone lets continue as is with now as separate components ill make another CU card to revisit the combination project at the end once we have everything in place.

pawelkmpt commented 1 year ago

@kertuilves it seems like you squashed your changes with existing commit during rebase. Please correct it. Are you familiar with git reflog?

pawelkmpt commented 1 year ago

@kertuilves you pushed wrong code here cxl-stats

kertuilves commented 1 year ago

@kertuilves you pushed wrong code here cxl-stats

@pawelkmpt The cxl-stats has been merged to feat/dashboard and when I rebased this branch from feat/dashboard and solved the merge conflicts it was supposed to come. No?

pawelkmpt commented 1 year ago

@pawelkmpt The cxl-stats has been merged to feat/dashboard and when I rebased this branch from feat/dashboard and solved the merge conflicts it was supposed to come. No?

Everything is fine. My bad. I looked at Compare option https://github.com/conversionxl/aybolit/compare/9f2bfc79dfe1adc13fb54768514c200119990f29..c9487b473530c54471bd36a12314245061c9e8c6, shown stats component there and I had an impression it's pushed here.

Sorry for that

pawelkmpt commented 1 year ago

@kertuilves taking into account Slack threads about the way of developing components:

I think we need change in this code. @freudFlintstone has almost completed <cxl-course-card> and you can look at his code here: https://github.com/conversionxl/aybolit/commit/3e46d1286e2d7c4a8b457581a4b58b42be0f679d#diff-5e748d1f0301dffd4bb4521b9dbe089c8a76e34023e06745e8848cd542447a6e

Main difference: component has set of attributes and properties which are encapsulated and rendered in specific way.

Good thing is that you can re-use styles you already created :)

What's next:

kertuilves commented 1 year ago

@pawelkmpt I created a separate component for light-card. How should I pass the badges data? Do we know how will the different roadmaps themes be defined? (will work on it after vacation)

kertuilves commented 1 year ago

Navigation differs from Figma design

I made the slider changes now here, too. But this was already discussed above that those slider styles were already under #288. And the slider here was just to show how multiple cards together would behave.