feat: add dashboard lightweight card #289

kertuilves · 2023-07-05T08:40:04Z

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

github-actions · 2023-07-05T08:41:49Z

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 · 2023-07-05T08:43:38Z

@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 · 2023-07-05T08:52:11Z

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,

so unclear how it would function with multiple cards

Additionally

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

pawelkmpt

Icon is cut

packages/cxl-lumo-styles/src/icons.js

packages/cxl-ui/scss/global/cxl-card.scss

pawelkmpt · 2023-07-05T10:17:46Z

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 · 2023-07-05T11:52:09Z

Hey @heshfekry.

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

heshfekry · 2023-07-05T12:01:45Z

Hey @heshfekry.

I modified the Course Card canvas layout and now the icon is visible

Course Card now has another with-roadmaps state

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 · 2023-07-05T12:08:52Z

@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 · 2023-07-05T12:15:22Z

@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 · 2023-07-05T12:24:16Z

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

packages/storybook/cxl-ui/cxl-card/[theme=cxl-course].stories.js

pawelkmpt

Looking good. Concerns about logic.

packages/storybook/cxl-ui/cxl-card/[theme=cxl-course].stories.js

packages/cxl-lumo-styles/package.json

packages/cxl-ui/src/components/cxl-light-card.js

pawelkmpt

Navigation differs from Figma design

it is:

should be:

packages/cxl-ui/src/components/cxl-light-card.js

pawelkmpt · 2023-07-27T13:51:22Z

packages/cxl-lumo-styles/scss/badge.scss

+[theme~='badge'][theme~='secondary'] {
+  color: var(--lumo-primary-contrast-color);
+  background-color: var(--cxl-color-brand-blue);
+}
+


Please make it separate commit as it's badge feature

@kertuilves following it up

kertuilves · 2023-08-01T10:13:35Z

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.

pawelkmpt

Please put badge code into one commit and rest into another

pawelkmpt · 2023-08-01T11:34:33Z

packages/cxl-lumo-styles/scss/badge.scss

+[theme~='badge'][theme~='secondary'] {
+  color: var(--lumo-primary-contrast-color);
+  background-color: var(--cxl-color-brand-blue);
+}
+


@kertuilves following it up

packages/cxl-lumo-styles/src/icons.js

packages/cxl-lumo-styles/scss/global.scss

pawelkmpt

Good job 👍
Next step: refactor to use base class

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

packages/cxl-ui/src/components/cxl-base-card.js

packages/cxl-ui/scss/cxl-base-card.scss

packages/cxl-ui/scss/cxl-time.scss

pawelkmpt

@kertuilves I marked couple of changes which should go into "improve base card" commit. For storybook templates it's not just a line I left the comment but all changes which are caused by showTimeIcon property introduction.

Light card itself looks good

packages/cxl-ui/scss/cxl-base-card.scss

packages/cxl-ui/src/components/cxl-base-card.js

packages/cxl-ui/src/components/cxl-course-card.js

packages/storybook/cxl-ui/cxl-course-card/[theme=playbook].stories.js

packages/storybook/cxl-ui/cxl-course-card/[theme=training].stories.js

packages/storybook/cxl-ui/cxl-course-card/[theme=video].stories.js

packages/storybook/cxl-ui/cxl-course-card/template.js

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

…card` https://app.clickup.com/t/861n0qm47

kertuilves requested review from lkraav, pawelkmpt and anoblet July 5, 2023 08:40

pawelkmpt requested changes Jul 5, 2023

View reviewed changes

packages/cxl-lumo-styles/src/icons.js Outdated Show resolved Hide resolved

packages/cxl-ui/scss/global/cxl-card.scss Outdated Show resolved Hide resolved

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch from 8d0f8ab to 86cae23 Compare July 5, 2023 11:42

kertuilves requested a review from pawelkmpt July 5, 2023 11:43

This comment was marked as outdated.

Sign in to view

pawelkmpt requested changes Jul 6, 2023

View reviewed changes

packages/storybook/cxl-ui/cxl-card/[theme=cxl-course].stories.js Outdated Show resolved Hide resolved

pawelkmpt mentioned this pull request Jul 6, 2023

feat: add dashboard category accordion #288

Merged

pawelkmpt changed the base branch from master to feat/dashboard July 6, 2023 08:05

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch from 86cae23 to 8746bea Compare July 6, 2023 10:33

kertuilves requested a review from pawelkmpt July 6, 2023 13:37

pawelkmpt reviewed Jul 7, 2023

View reviewed changes

packages/storybook/cxl-ui/cxl-card/[theme=cxl-course].stories.js Outdated Show resolved Hide resolved

anoblet requested changes Jul 8, 2023

View reviewed changes

packages/storybook/cxl-ui/cxl-card/[theme=cxl-course].stories.js Outdated Show resolved Hide resolved

packages/storybook/cxl-ui/cxl-card/[theme=cxl-course].stories.js Outdated Show resolved Hide resolved

packages/cxl-lumo-styles/package.json Outdated Show resolved Hide resolved

anoblet mentioned this pull request Jul 9, 2023

feat(cxl-ui): implement dashboard card design #293

Merged

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch 3 times, most recently from bf1c620 to 2bbc028 Compare July 10, 2023 18:23

kertuilves requested review from pawelkmpt and anoblet July 10, 2023 18:25

pawelkmpt requested changes Jul 14, 2023

View reviewed changes

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch 2 times, most recently from 3156038 to 81ee5d4 Compare July 14, 2023 14:51

kertuilves requested a review from pawelkmpt July 14, 2023 14:52

pawelkmpt added the CXL 2.0 label Jul 26, 2023

pawelkmpt requested changes Jul 27, 2023

View reviewed changes

pawelkmpt requested changes Aug 1, 2023

View reviewed changes

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch 3 times, most recently from d4b8555 to 95ac784 Compare August 1, 2023 12:57

kertuilves requested a review from pawelkmpt August 1, 2023 12:59

pawelkmpt requested changes Aug 2, 2023

View reviewed changes

packages/cxl-lumo-styles/scss/global.scss Show resolved Hide resolved

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch from 95ac784 to 6cbc90b Compare August 2, 2023 07:34

pawelkmpt approved these changes Aug 2, 2023

View reviewed changes

phkel added 2 commits August 2, 2023 10:48

feat(cxl-lumo-styles): add badge secondary theme

308b7a7

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

feat(cxl-ui): add cxl-light-card component

1fae565

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

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch from 6cbc90b to 1fae565 Compare August 2, 2023 07:49

kertuilves requested a review from pawelkmpt August 2, 2023 11:16

pawelkmpt requested changes Aug 3, 2023

View reviewed changes

packages/cxl-ui/src/components/cxl-base-card.js Show resolved Hide resolved

packages/cxl-ui/scss/cxl-base-card.scss Outdated Show resolved Hide resolved

packages/cxl-ui/scss/cxl-time.scss Show resolved Hide resolved

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch from 35b7b02 to bdb5cf1 Compare August 7, 2023 09:47

kertuilves requested a review from pawelkmpt August 7, 2023 09:48

pawelkmpt requested changes Aug 7, 2023

View reviewed changes

kertuilves added 2 commits August 7, 2023 16:52

feat(cxl-ui): improve cxl-base-card component

1a688fa

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

feat(cxl-ui): refactor cxl-light-card component based on `cxl-base-…

4989403

…card` https://app.clickup.com/t/861n0qm47

kertuilves force-pushed the kertu/feat/dashboard-lightweight-card branch from bdb5cf1 to 4989403 Compare August 7, 2023 13:53

kertuilves requested a review from pawelkmpt August 7, 2023 13:54

pawelkmpt approved these changes Aug 8, 2023

View reviewed changes

pawelkmpt merged commit 9fdc717 into feat/dashboard Aug 8, 2023
4 checks passed

pawelkmpt deleted the kertu/feat/dashboard-lightweight-card branch January 9, 2024 12:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add dashboard lightweight card #289

feat: add dashboard lightweight card #289

kertuilves commented Jul 5, 2023

github-actions bot commented Jul 5, 2023 •

edited

Loading

heshfekry commented Jul 5, 2023

heshfekry commented Jul 5, 2023 •

edited

Loading

pawelkmpt left a comment

pawelkmpt commented Jul 5, 2023

kertuilves commented Jul 5, 2023

heshfekry commented Jul 5, 2023

kertuilves commented Jul 5, 2023

heshfekry commented Jul 5, 2023

kertuilves commented Jul 5, 2023

This comment was marked as outdated.

This comment was marked as outdated.

pawelkmpt left a comment

pawelkmpt left a comment

pawelkmpt Jul 27, 2023

pawelkmpt Aug 1, 2023

kertuilves commented Aug 1, 2023

pawelkmpt left a comment

pawelkmpt Aug 1, 2023

pawelkmpt left a comment

pawelkmpt left a comment •

edited

Loading

feat: add dashboard lightweight card #289

feat: add dashboard lightweight card #289

Conversation

kertuilves commented Jul 5, 2023

github-actions bot commented Jul 5, 2023 • edited Loading

size-limit report 📦

heshfekry commented Jul 5, 2023

heshfekry commented Jul 5, 2023 • edited Loading

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt commented Jul 5, 2023

kertuilves commented Jul 5, 2023

heshfekry commented Jul 5, 2023

kertuilves commented Jul 5, 2023

heshfekry commented Jul 5, 2023

kertuilves commented Jul 5, 2023

This comment was marked as outdated.

This comment was marked as outdated.

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt Jul 27, 2023

Choose a reason for hiding this comment

pawelkmpt Aug 1, 2023

Choose a reason for hiding this comment

kertuilves commented Aug 1, 2023

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt Aug 1, 2023

Choose a reason for hiding this comment

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 5, 2023 •

edited

Loading

heshfekry commented Jul 5, 2023 •

edited

Loading

pawelkmpt left a comment •

edited

Loading