New component: Initial implementation of KCard component

[AllanOXDi]

Description

This PR introduces the KCard component to KDS, It will act a foundational building block for creating various card types within our product eco system. By serving as a base component for creating different card types like lesson cards, resource cards, and channel cards.

Issue addressed

#530
#528
#529

Changelog

• [New component: Initial implementation of KCard component #625]
• Description: Initial implementation of KCard component
• Products impact: New Component
• Addresses: #530
• Components: KCard
• Breaking: No
• Impacts a11y: Yes

Steps to test

I suggest looking at these issues
#530
#528
#529

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Critical and brittle code paths are covered by unit tests
• The change is described in the changelog section above

Reviewer guidance

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all UI components LTR and RTL compliant (if applicable)?
• Add other things to check for here

After review

• The changelog item has been pasted to the CHANGELOG.md

Comments

[AllanOXDi]

I've added a few cards to the playground for @radinamatic to perform QA testing. Once everything is approved by the QA team, we'll remove the cards before merging the code.

[radinamatic]

Overall very good implementation, you followed all the recommended accessibility guidelines 👏🏽 💯 🙂

One thing that needs to be corrected is the use of the semantic header and footer elements inside cards. We should have been more clear in the component specification, but the fact that we call something card footer and card header does not imply that semantic header and footer landmark elements need to be implemented. These are page-level elements and having them (announced) for each of potentially tens of cards on the page would be confusing.

Header	Footer

[radinamatic]

Regarding the responsiveness, I don't know if it's because of Netlify view of the playground (🙃) but I'm wondering if it's how it's supposed to look and behave...? Or maybe we can only properly test this once the cards are in the grid? 🤔

Untitled.Project.mp4

[radinamatic]

The last question I had was regarding the other interactive elements inside the card (button/icon to open other options, etc.): that is not in the scope of this PR and is supposed to come in subsequent ones, correct?

[AllanOXDi]

The last question I had was regarding the other interactive elements inside the card (button/icon to open other options, etc.): that is not in the scope of this PR and is supposed to come in subsequent ones, correct?

I think it's something we tried and tested here learningequality/studio#4480

[AllanOXDi]

but I'm wondering if it's how it's supposed to look and behave...? Or maybe we can only properly test this once the cards are in the grid? 🤔

I think that's how it should behave for now.

Regarding the responsiveness, I don't know if it's because of Netlify view of the playground (🙃) but I'm wondering if it's how it's supposed to look and behave...? Or maybe we can only properly test this once the cards are in the grid? 🤔

Untitled.Project.mp4

The Grid will control it's behavior further . I will defer to @MisRob for more thoughts and guidance on this.

[radinamatic]

I think it's something we tried and tested here learningequality/studio#4480

I don't recall having tested that, is it live somewhere for me to take a look?

[AllanOXDi]

I don't recall having tested that, is it live somewhere for me to take a look?

Not- it's not yet live for testing as dev is still on going.

[MisRob]

Thank you @radinamatic and @AllanOXDi.

Regarding responsiveness - that's something that a grid will control, so yes, it is to be implemented and tested later on.

Regarding buttons in the footer area - KCard itself only provides place for them. It is products that will be responsible for adding buttons, context menus, etc. and ensuring it doesn't break a11y. We tried it a bit in a Studio PR as @AllanOXDi mentioned, but that's work in progress and there should be more thorough QA any time we add new cards in a product. After we start seeing some patterns, we may also consider adding some guidance to the documentation or adding features to KCard to further support such cases.

[radinamatic]

All good then, will wait for the refactor of semantic header and footer landmark elements here to approve, and looking forward to testing the PRs for follow up steps of the new KCard! 🙂

[AlexVelezLl]

Hi @AllanOXDi! I am glad that our KCard is already taking shape 🎉. I noticed a few things, especially in the documentation, and in the KCard code structure. Let me know any thoughts of any of the comments 👐.

[AlexVelezLl]

I think this title slot is an alternative to the title prop. So I think we will probably want to make this part of the BaseCard component perhaps?

[AllanOXDi]

That would not be a good idea. I think BasCard is to enforce consistent structure and accessibility for cards across our product and Isolates basic accessibility requirements from potential changes in the KCard component,

[MisRob]

BaseCard is meant to have the validation "Contains validation to check that title prop or slot has been used, throwing an error if neither is provided" (see acceptance criteria here #529) so I think the slot will need to be present in the BaseCard. It will add some clarity as well. This is part of the enforcement that BaseCard does - it needs to ensure that title is provided, and that can happen either via the prop or the slot.

@AllanOXDi you too have made a good point and regarding that, we need to ensure that BaseCard shouldn't have any styles or layout logic related to how the title appears.

[MisRob]

Wonderful work overall @AllanOXDi, and it's great that a11y QA passes, that was the most tricky thing!

I am posting some first feedback. I haven’t reviewed yet everything - will follow up gradually.

[MisRob]

[required] I believe this is unused file that can be removed

[MisRob]

[required] title slot is to be an alternative to the title prop, however now the slot renders in addition to the title slot and is not showing in a correct place. For example, for

<KCard
    :to="{ name: '/' }"
    :headingLevel="2"
    layout="vertical"
  >
    <template #title>
      Card title
    </template>
</KCard>

I see the title text not being rendered as heading:

Could you adjust the behavior to follow the expected behavior in all of the following cases? Note that in one of them it works well already, but not for all - including all for clarity. There's also an additional issue in the screenshots below regarding the placement of the aboveTitle but I believe it is already being addressed.

	Expected
only title prop <KCard title="Card title">
only title slot <KCard><template #title>Card title</template></KCard>
both title prop and title slot <KCard title="Card title from the prop"><template #title>Card title from the slot</template></KCard> - give preference to the title prop and ignore the slot

[MisRob]

[required] One important area to address here is to check the resulting UI against the "Updated Cards Specs Metadata" designs and make sure that all the logic and styles that KCard is responsible for aligns to them (while leaving out areas that KCard is not responsible for, such as content of slots like icons, buttons, etc.) Few examples:

Design	Implementation

These are just a few places I noticed, and is not meant to be a full list. Could you please take some time to test all available layouts and compare with designs? Placement, font sizes, padding/margins, and pretty much anything we can do to follow designs closely. Figma will provide you with all information and useful measurement tools, such as

Note that letterboxing should be preserved though even though it's not showed on the designs. The image on the designs corresponds to the whole thumbnail area (possibly letterboxed) in the final implementation.

[MisRob]

@AllanOXDi There are some acceptance criteria from the issues this PR is meant to close that appear not yet be implemented or have unclear status. Could you please read through all related issues, particularly "Markup/style requirements" and the "Acceptance criteria" sections and resolve them or mention where things are if there are any blockers? Some examples: "Supports RTL", "Even though the whole area is clickable, text content is still selectable", "[documentation] contains guidance on the headingLevel to notify developers that they need to choose a correct level based on context", and perhaps few more.

I know it's lots of work, but there's time to finish it and would suggest we do so in this PR since it's not draft like the first Studio PR. I think it will be easier than opening many follow ups since there's no pressure around merging. This will prepare robust ground for further work and help with implementing card grids as well. If you needed to chat around how to organize work or encountered some blocking issues, we'll have time to chat :)

Resolved

[MisRob]

Hi @AllanOXDi, I finished the final review. Overall it feels much more robust now, layouts look great, and I really appreciate you took time to prepare cards preview similar to the cards we will implement in Kolibri. I am sure this will make all further work in products smooth. Thank you for all your hard work :)

As we chatted earlier, it'd be great to have this merged before you're back online and open follow-ups if there's some more work. I will open a few, but for some changes, it's a bit easier for me to push them right away rather than explaining in new issues, and I also hope it will help with other work we have in progress in Kolibri and Studio that you can return to when you are back. I hope you don't mind - it's done with the intention of support, and hope it aligns well with all we talked about together. I'm trying my best to describe reasons for changes in the commit messages, and will be available to talk about each of them. The main areas updated I considered important (cc @AlexVelezLl for review):

User-facing:

• Fixes for some last issues with the title slot
• Consistency of the title styles no matter if title provided via prop or slot
• Improvements to styles robustness and content tolerance, and some last styling touches
• Ensuring that thumbnails can display placeholders via "thumbnailPlaceholder" slot when thumbnail is not available

Other:

• Re-organization of styles in KCard regarding various layouts so that styles belonging to particular layouts are grouped together
  • This is really not so important for users, however I believe it will help with future development, and it also helped me a lot with some styling updates I am mentioning above. I think we had this done already to some extent as response to @AlexVelezLl's first review, but seems to have gotten lost with big styles overhaul after fixes to markup structure. It's lots of changes but rather organizational with a few fixes applied, the foundational techniques you put in place stay the same.

(You will also see some documentation and renaming mixed in my commits and that's just me nitpicking and doing little improvements when I see an opportunity as I'm in the code already, nothing really important, and in majority of cases, I wouldn't be posting to review :)

After I am done, I will ask @AlexVelezLl to review my updates, and then we will merge. Thanks both and talk to you soon.

[MisRob]

Would you please give the components one last look, @AlexVelezLl? Please also see the playground page that simulates more use-cases now. I will delete it before merging, but keep a copy somewhere for testing (this will be perfect first task for automated visual tests :)

Also please note that the following will be resolved in follow-ups (some already open, and I will open later remaining ones). If there's any feedback from you that's not resolved and not blocking, please mention.

• Clarify how cards will behave in a grid regarding their height with designers, e.g. should the cards height be always the same or should the row adjust to the highest card (current implementation is my best guess)? Based on that, determine if we need to be able to override cards heights/widths, remove spaces occupied by empty slots, etc. (I will do this as part of grid work)
• Some more guidelines in documentation for frequent tasks (probably me)
• Vertical/horizontal layout with no thumbnail (me or Allan)
• Adding constants for layout and thumbnaiDisplay and related prop validators (Allan)
• Implementing RTL support (Allan)

[radinamatic]

I took a look at the latest version of the playground and tested with NVDA in Windows.

One thing popped up as a concern, but again I'm not sure if it's determined by this base card implementation, or will it be further injected by other products. Namely, the color contrast of the channel name (is that in the aboveTitle slot?) is insufficient:

Similarly, even though I understand what was mentioned earlier, that:

products that will be responsible for adding buttons, context menus, etc. and ensuring it doesn't break a11y.

I just wanted to reiterate 🙂 , that we need to pay attention to offer full and comprehensive information about those element in the footer. For example, this is the full readout of one of the cards by NVDA:

clickable  
<Title>  
link    
heading    level 2  
<Below title>
PracticeShort Activity  20

All above looks Ok, just 2 comments regarding the last line:

• a separation is missing between the type of resource and the duration
• progress element needs a role and a label, otherwise the screen reader outputs just the number (20) without any context

I know this might not be in the scope of the present PR, but it cannot hurt to repeat it 😉

[AlexVelezLl]

This LGTM! Code looks good and well documented, documentation looks very informative and easy to follow, and visual examples in the playground looks great! I think is ready to go! Just left a super minor comments about docs.

Thank you @AllanOXDi for all your consistent hard and great work here!! and @MisRob for your great guidance! Will leave the final merge to @MisRob once playground is cleaned and QA passes.

[AlexVelezLl]

I see value in adding that this allows us select text for example.

[MisRob]

Ah yes, it was couple of lines above - will make sense to have it here together. Updated, thank you.

[AlexVelezLl]

I think we should document these slots as well.

[MisRob]

Ah yes, thank you. Done

[MisRob]

Thank you @radinamatic, I appreciate it. Yes, these are all the things that consuming apps need to ensure as they will inject it. I'd like us to have some guidance into the documentation to clarify for developers what we need to take care of when implementing cards on top of this component vs what is it already taking care of. Also, as soon as we have first few Kolibri card components implemented and a11y tested, we could use the way they look for the documentation examples to reflect this all a bit better.

[MisRob]

^ We will keep working on this all gradually as part of the project, this component is a core step, and also a beginning of larger body of work in a sense.

[MisRob]

Thanks for feedback @AlexVelezLl! Merging now.