🐛 [BUG] - PricingOptions.FeatureListGroupHeading missing TS definition

[seangolob]

Describe the bug

While the as prop works for PricingOptions.FeatureListGroupHeading. The TS compiler is complaining that as is not supported for this component.

I think we need to adjust https://github.com/primer/brand/blob/main/packages/react/src/PricingOptions/PricingOptions.tsx#L516

to support HeadingProps as well.

Reproduction steps

1. Create a minimal pricing options component
2. Add the following code to recreate the TS error


<PricingOptions.FeatureListGroupHeading as="h6">
  Pricing Options Feature List Group Heading with H6 
</PricingOptions.FeatureListGroupHeading>

Expected behavior

Despite this code working fine in runtime, typescript throws an error.

Screenshots

Browsers

No response

OS

No response

[seangolob]

As this is purely a DX issue, I am unsure if bug is the correct label for this. Let me know if this is tagged appropriately.

Also, we have a workaround in place so this is not blocking work.

[danielguillan]

Thanks for the report, @seangolob! This is a quick fix, so we'll include a fix in the next release.

[seangolob]

@danielguillan Another issue I noticed is that when you pass as={undefined}, it causes the heading level to use the default heading level of the Heading component (which is h2). Similar to the other headings in this component, we should programmatically declare a default value of h4.

We have a workaround in place that looks like:

const featureListGroupHeading = feature as PrimerComponentPricingOptionsFeatureListGroupHeading
const featureListGroupHeadingLevel = featureListGroupHeading.fields.headingLevel

// Only spread the `as` prop if a heading level is defined;
// passing `as={undefined}` causes the incorrect heading level to be rendered.
const props = featureListGroupHeadingLevel
  ? { as: featureListGroupHeadingLevel }
  : {}

return (
  <PricingOptions.FeatureListGroupHeading {...props} key={feature.sys.id}>
    {documentToPlainText(featureListGroupHeading.fields.heading)}
  </PricingOptions.FeatureListGroupHeading>
)

Let me know if more context is needed!

[danielguillan]

Hey! @seangolob, regarding the as={undefined} issue, we're curious why you need to set the value to undefined instead of simply omitting the prop, which would render the correct heading level. Could you please provide more context on the use case?

[seangolob]

Sure thing! For every Primer Brand component we support, we have a reusable model in Contentful. So we may have 50 pricing options components in Contentful that all use the same ContentfulPricingOptions component code in Dotcom.

If you check out our source code for our wrapper for the Pricing Options component, you can see that we pull all of the fields from Contentful and plug them into props of the various pricing option components.

A lot of these values that are destructured off fields are undefined. This is essentially the Contentful editor not leveraging the field. When we build components in Contentful, we try to surface as many props as realistically possible, to give the most flexibility when building pages.

Happy to give more context!