fix(typography): unintended typography element margins

[saashapina]

Description

Removes --spectrum-detail-margin-start and --spectrum-detail-margin-end definitions from .spectrum-Detail and .spectrum-Heading ensuring margin custom properties are scoped only to .spectrum-Typography .spectrum-Heading and .spectrum-Typography .spectrum-Detail.

Motivation and context

We recently made a change in the Typography CSS that unintentionally scoped margin custom properties more broadly than intended. Typography margin custom properties should only be scoped to .spectrum-Typography

Related issue(s)

• fixes CSS-1248

Author's checklist

• I have read the CONTRIBUTING and PULL_REQUESTS documents.
• I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
• I have added automated tests to cover my changes.
• I have included a well-written changeset if my change needs to be published.
• I have included updated documentation if my change required it.

---

Reviewer's checklist

• Includes a Github Issue with appropriate flag or Jira ticket number without a link
• Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
• Automated tests cover all use cases and follow best practices for writing
• Validated on all supported browsers
• All VRTs are approved before the author can update Golden Hash

[changeset-bot]

⚠️ No Changeset found

Latest commit: b1f0645

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

📚 Branch Preview

• Documentation Site
• Storybook

🔍 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

• Spectrum | Light | Medium | LTR
• Spectrum | Dark | Large | RTL
• Express | Light | Medium | LTR
• Express | Dark | Large | RTL
• Spectrum-two | Light | Medium | LTR
• Spectrum-two | Dark | Large | RTL
• High Contrast Mode | Medium | LTR

Deployed to Azure Blob Storage: pr-5576

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

[github-actions]

Tachometer results

Currently, no packages are changed by this PR...

[rise-erpelding]

I'm not really sure if there's a great way to test this, but here's what I did when I was initially investigating the bug:

1. Go to one of the Divider component's stories within Storybook
2. Inspect the heading component there, confirm that it does have top and bottom margins
3. Remove the .spectrum-Typography class from the parent div, confirm that there are no margins

So far this works fine! But if I change the .spectrum-Heading class into .spectrum-Detail to test the same with the Detail component, it doesn't work. The detail has margins both with the parent .spectrum-Typography class and also without it, which isn't expected behavior, so I'm wondering if we missed something somewhere.

On the original PR that introduced the issue (#5295), I see that the same --spectrum-heading-margin-start/-end being added to .spectrum-Heading rather than only .spectrum-Typography .spectrum-Heading being introduced in tools/styles/fonts.css as well, so maybe we need to roll those changes back also? Although the margins work just fine for heading so I find that puzzling 🤔 See https://github.com/adobe/spectrum-web-components/blob/saasha/fix-typography-margin/tools/styles/fonts.css#L76-L77

Likewise, that same PR does the same thing for .spectrum-Detail margins in that fonts.css file, see https://github.com/adobe/spectrum-web-components/blob/saasha/fix-typography-margin/tools/styles/fonts.css#L505-L506, so maybe that can be reverted back to what it was before also?

I also see the same being introduced in that PR in tools/styles/typography.css, see https://github.com/adobe/spectrum-web-components/blob/saasha/fix-typography-margin/tools/styles/typography.css#L76-L77 and https://github.com/adobe/spectrum-web-components/blob/saasha/fix-typography-margin/tools/styles/typography.css#L505-L506

Not sure if that would create any effect that we'd notice locally or in the branch deploy urls, but it would at least fully roll back the change? 🤷‍♀️

[saashapina]

Not sure if that would create any effect that we'd notice locally or in the branch deploy urls, but it would at least fully roll back the change? 🤷‍♀️

Good catches! I've rolled back these additional areas where the --spectrum-heading-margin-start/-end custom variables were being applied to the .spectrum-Heading/Details class and I ran an entire repo search to ensure they've all been removed from those classes but for some reason they're still showing up in the browser 🤔 Is this still happening on your end as well with the latest changes?

[rise-erpelding]

Looks great!