Componentize image card

[ashimupd]

[netlify]

❌ Deploy Preview for accessiblecommunity failed.

Name	Link
🔨 Latest commit	53fcecf
🔍 Latest deploy log	https://app.netlify.com/projects/accessiblecommunity/deploys/695f66fec436a000088691d4

[courtneylinder]

What are your thoughts of instead of having all the individual classes for border, title, col, we create variants of this ImageCard? I would maybe need to check the visuals so we could see what to call them/how to group them but in a Design System there is a balance for sure of allowing anything to go and having some constraints by saying like hey we have these 2-3 variants, please use one of those

[ashimupd]

Oh yeah, I've seen that approach in systems like MUI, where a predefined set of styles is applied to the component based on use cases that the D/S has already identified, but unsure if we have done that exercise yet -- I'll resolve this now and create a new issue to track this suggestion to discuss / implement next.

[courtneylinder]

Since you set this class by default, you shouldn't need to pass it in your instances of this in other files.

[brian-montgomery]

I would set the default border based on the theme, like you do the titleClass. Something like as a second block works well (and means you don't have to declare an extra variable).

const { 
    borderClass = `border-${theme}-subtle`, 
    titleClass = `text-${theme},
} = Astro.props;

[ashimupd]

Thanks @courtneylinder and @brian-montgomery I changed it to be set based on theme

[courtneylinder]

Thoughts @brian-montgomery on button vs link text? We are making an anchor tag but we use btn bootstrap styling classes

[brian-montgomery]

I don't have a strong opinion. It looks like a button with the styling, but there's not a form or anything, so it more functions more like a link.

[ashimupd]

Since it takes the user to another route, I thought of keeping it as an anchor semantically -- please let me know if we need to discuss further -- I'll resolve this comment for now

[brian-montgomery]

I apologize that it's taken me so long to get to this. It's a great start, let's discuss a little bit and either (a) fix the work here or (b) submit this and create some issues based on the suggestions and discussion that come out of this.

[brian-montgomery]

I would set the default border based on the theme, like you do the titleClass. Something like as a second block works well (and means you don't have to declare an extra variable).

const { 
    borderClass = `border-${theme}-subtle`, 
    titleClass = `text-${theme},
} = Astro.props;