Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add locked state to cards #431

Merged
merged 1 commit into from
Aug 19, 2024
Merged

feat: add locked state to cards #431

merged 1 commit into from
Aug 19, 2024

Conversation

anoblet
Copy link
Collaborator

@anoblet anoblet commented Aug 13, 2024

https://app.clickup.com/t/86b1nztyh

Course card:

image

Light card:

image

@anoblet anoblet requested review from lkraav and pawelkmpt August 13, 2024 16:15
Copy link

github-actions bot commented Aug 13, 2024

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 44.75 KB (+1.15% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.89 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 29.23 KB (+0.78% 🔺)
packages/cxl-ui/pkg/dist-web/vendor.js 138.23 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-institute.js, 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 277.57 KB (+0.41% 🔺)

@heshfekry
Copy link

heshfekry commented Aug 13, 2024

Hey @anoblet

Good start. What you showed above would be difficult for users as they can not see the title of the course, so they will just see a list of CTAs and the same microcopy. Original intention was to swap the excerpt.

What you showed however could work on hover. Something like this?

Normal state
image

On-hover state
image

I would probably keep the CTA to take the user to the course. As some have promo videos and they may want to find out more about the course. They can always click this piece on the course page to go to pricing.

image

@heshfekry
Copy link

heshfekry commented Aug 13, 2024

the original intention was to put this copy

image

here

image

Maybe we can try both and see what looks/feels better? can you proivde a storybook link of both please?

@heshfekry
Copy link

@anoblet if you are struggling to get hover to work properly, then we can just try originally intended version.

Copy link

@pawelkmpt pawelkmpt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is overcomplicated. Also such message should not be hardcoded. It was supposed to replace regular course description.

@lkraav
Copy link

lkraav commented Aug 14, 2024

"All-Access" product name always hyphenated, right?

Unsure: do we need to repeat CXL? I mean who else's All-Access are we selling?

@heshfekry
Copy link

heshfekry commented Aug 14, 2024

Unsure: do we need to repeat CXL? I mean who else's All-Access are

BRAAAANDDDDDD!!!!

I agree and am definitely up for removing repetition here, its redundant. Especially in a tooltip which is latest incarnation proposed by @pawelkmpt

image

would propose a slight tweak to the tool tip copy as it conflicts with the CTA. Purchase to view >> View course

Purchase an All-Access subscription to unlock this course.

@pawelkmpt
Copy link

Screenshot 2024-08-14 at 12 06 50

@pawelkmpt
Copy link

Also such message should not be hardcoded.

Made it hardcoded as @heshfekry suggested message different than we currently use but provided override possibility.

@pawelkmpt pawelkmpt force-pushed the anoblet/feat/locked branch from 5b3241d to ed44713 Compare August 14, 2024 11:39
@pawelkmpt
Copy link

cxl-light-card locked

Screenshot 2024-08-14 at 14 33 03

@pawelkmpt pawelkmpt force-pushed the anoblet/feat/locked branch from b3015dd to b22795a Compare August 19, 2024 11:13
@pawelkmpt pawelkmpt merged commit dfb02e1 into master Aug 19, 2024
5 checks passed
@pawelkmpt pawelkmpt deleted the anoblet/feat/locked branch August 19, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants