feat(accordion-item): custom header and content spacing tokens

[alisonailea]

Related Issue: #4012

Summary

Add spacing

Removes pt-0 from .content to allow spacing between the header-content and content.

Before

After

[ashetland]

Does this address the padding around the content slot or only add a token to the header?
Fixed

[ashetland]

Visual changes lookin' good!

[macandcheese]

Do we need this header space token to be public? I'd expect only the main content area to have its padding adjusted in this manner. I think we could solve just by adjusting the existing implementation to not use the public property on the header.

[alisonailea]

Do we need this header space token to be public? I'd expect only the main content area to have its padding adjusted in this manner. I think we could solve just by adjusting the existing implementation to not use the public property on the header.

@macandcheese see Paul's comment here - #4012 (comment)

[macandcheese]

Do we need this header space token to be public? I'd expect only the main content area to have its padding adjusted in this manner. I think we could solve just by adjusting the existing implementation to not use the public property on the header.

@macandcheese see Paul's comment here - #4012 (comment)

Maybe we could confirm? I’m just interpreting that as needing to configure the content area padding without that also affecting the header?

[ashetland]

Do we need this header space token to be public? I'd expect only the main content area to have its padding adjusted in this manner. I think we could solve just by adjusting the existing implementation to not use the public property on the header.

@macandcheese see Paul's comment here - #4012 (comment)

Maybe we could confirm? I’m just interpreting that as needing to configure the content area padding without that also affecting the header?

I may have misread Paul's comment as asking for separate control. The original issue only asked for content slot padding and, reading Paul's comment again, I agree with Adam's assessment. This would generally match what we're doing elsewhere, right?

[alisonailea]

@macandcheese and @ashetland I've updated the PR and removed the header token. I agree this makes more sense

[macandcheese]

This is still applying to the .header-content div, at line 78:

.header-content,
.content {
  padding: var(
    --calcite-accordion-item-content-space,
    var(
      --calcite-internal-accordion-item-padding,
      var(--calcite-internal-accordion-item-spacing-unit, theme("spacing.2") 0.75rem)
    )
  );
}

That div should just use the --calcite-internal-accordion-item-padding and not the public one.

[alisonailea]

it is currently using the public property on /dev. Are you saying that should not have the same padding as the content?

[macandcheese]

Yeah - the issue in the original issue is that changing the content also affects the header. All the other instances where we have a "content-padding" or equivalent property, it just affects the content area. Here I'd expect the heading padding to remain unchanged when a user adjusts the "content-space" property.

[macandcheese]

I believe the .header-content needs to be separated from .content to ensure the public css property doesn't apply.

I think we should also mark this as a breaking change (or at least visual change). Users can of course retain the previous design by using the public css property and it's minor but just for logging purposes.

[alisonailea]

There were some weird merge conflicts with /dev so I cleaned up the PR to only include the removal of the 0px top padding as was requested in the original issue. If we want to separate the header and content padding that should be a separate issue.

[macandcheese]

Looks good - maybe just a tracking issue for the content space comment. Thanks!

[alisonailea]

Looks good - maybe just a tracking issue for the content space comment. Thanks!

#10762