Sample component too look like the design specs

At the moment, we're applying padding on the Card component instead of each child component for ease of implementation reasons. However, this means the users will need to revert this padding if they so have such a specific need, so perhaps having a spacing token and reusing this across all Card* components is a better way to go.

To be extracted into a separate utility once we define our standard of interactivity.

I would avoid making styling assumptions based on event listeners used as it might lead to higher amount of custom overrides. Having explicit prop like interactive?: boolean is more robust. You can consider changing the default value of it based on the listeners (like if onClick is used and interactive is not, it would still be interactive, unless consumer specifies interactive={false})

I don't see what the point of an interactive prop would be if the styling is still applied based on events. To me this is a visual accessibility feature that we want to be there.
If the user wants actions with no visual feedback or vice-versa, this seems like behavior we wouldn't want to encourage.

@jurokapsiar That's a valid point and even more of a reason to have this logic abstracted into a separate utility which can reason about it internally based on state.

I am, however, a bit afraid of a prop like interactive?: boolean as it exposes styling internals. This will also mean we run into a situation where we decouple styling interactivity and functional interactivity.

• What should happen if the prop is true but there are no handlers?
  • Should it look interactive but when users try to manipulate it, it does nothing?
• What should happen if the prop is false and there are handlers?
  • Should it disable those handlers?
  • Should it look non-interactive? But how are we then handling cases like keyboard focus?

If we believe that current component spec will lead to high amount of custom overrides then in my opinion the component design is not done properly and interactivity should be better defined within the spec. Otherwise, this would just be a premature optimisation for a problem that might not be a problem at all, especially when taking into consideration there already is a mechanism for users to override the styling if they want to opt-into less common scenarios.

Looking at the figma design specs, there is no requirement that these interactive styles are linked to any kind of event listeners.

Why does interactive necessarily mean a mouse click ? why can a mouseenter not define interactive behaviour ? If a user wants to have hover styles, they would currently need to add an empty click listener to do that.

The event listeners also miss out on contextmenu event, should that also define interactivity ?

Adding an interactive prop means that we allow maximum flexibility for users since the designers who will work with these components can be from different product groups and might not associate things like hover styles with a click. On the reverse, some designs might not want hover styles or pressed styles to be applied on the card even on click, the simple prop lets us quite easily handle both situations without being too hacky

@andrefcdias @jurokapsiar @ling1726 I opened an issue which covers this. I feel this discussion targets more than just the card and feel that extra visibility of a separate issue might be useful. If you could raise your points there it would be greatly welcome. 🙏

#19664

This is why we need to define interactivity. My personal opinion is that if something has an action, it's interactive and needs visual feedback of being actionable. A mouseenter event is not an event that, to me, would define interactivity.
End of the day, this is something that should be opt out. If the user doesn't define anything, we provide them with a good baseline.

My suggestion is that we go with the current solution, keeping in mind that this is definitely not set in stone. There's already the pretext that breaking changes will happen in the next phases, so let's not dwell on this right now and migrate this discussion into the spec PR to define the definite solution for it, unless this is of high importance to have defined right now.
What do you think?

you'll need to make sure these aren't released along with our published packages. I would suggest you update the migration generator to update the generated npmignore for either:

• assets folder -> could be a bit too opinionated and specific to one package
• all svgs -> I don't think we should be shipping any svgs, but I'm not 100% sure on this

Should I just use @fluentui/example-data instead or is this a v8 thing only?

I think it's a v8 thing, but I don't think including it as a dev dep should have too many consequences just like other shared dev deps between v8/9. @ecraig12345 any reason we can't use that package as a dev dep for stories ?

since this is the first phase of a new component, please follow the new slot guidelines from #19483 It's a beta high priority task to migrate components to the new API

Compat was used on purpose as the PR was still pending approval back then. If this is not breaking, I would rather leave it for when the implementation of the rest of the functionality starts to avoid delaying it (given that it's not one of the Beta components and we're not closing Card with this).
Could this be postponed to the next PR?

I would say that when there's less functionality this change is easier to make (having had to do this for the full Menu package). But you are driving the work on card, so I'll leave it to your timeline 👍

Plan would be to pick that up right after the PR gets merged and before we start with additional functionality, so impact should be the same.

I also don't like this, but every DOM slot should be customizable. In MenuItem I had the same issue and just settled for a content slot

I see now that you do have a content slot, did you forget to use it in the render function ?

It's under renderCardHeader.tsx

so the same should for done for renderCardFooter ?

I see the confusion now. I thought this was referring to CardHeader and not CardFooter. 😅
I think I would prefer to have slightly more complex styling to achieve the same result instead of having a content prop for this case. It's only there so we have content to the left and actions to the right.
Opinion?

why not just use flex display and set margin-left: 'auto' on the actions slot ? I checked out your PR to test it out, it seems to work

I went ahead with creating a content slot instead. margin-left would not work for RTL scenarios and there doesn't seem to be any viable solution for it. There's the :dir psudo-selector but that's not fully supported yet.

I'm pretty sure sure make-styles does RTL styles flipping automatically

I've reverted the API changes back to leverage that functionality. Let us discuss going forward over the best approach given that this depends on knowledge about the FluentProvider

are we sure that we want to release immediately ? are we in a rush to deliver this to partners ?

as you said it's not targeted for the beta release

Partner's needs are basically ASAP, hence the need for this first phase iteration.

Another note: if partners consume this as a separate package along with react-components there would be compatibility issues since the removal of mergeProps and other breaking changes to slots. It would be hard for them to figure out why things are breaking because we use prerelease tags that don't communicate breaking changes

Should the image be required?

Yes, at least for now. We don't really have any designs at the moment for scenarios with missing pieces and making it optional in the future wouldn't bring breaking changes.