Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(form): replace use of context to apply field spacing #7243

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nuria1110
Copy link
Contributor

fix #7058

Proposed behaviour

  • Replaces FormSpacing context with css only solution to ensure margins are applied across projects.
  • Character count in components using Textbox receives margin.

Current behaviour

  • Form uses FormSpacing context to apply spacing to vertically separate inputs. This causes issues for consumers using Form in exported components, as the spacing is not applied across projects.
  • Margin-bottom is not applied to character count in components using Textbox.
    image

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

Testing instructions

@nuria1110 nuria1110 self-assigned this Mar 6, 2025
@nuria1110 nuria1110 marked this pull request as ready for review March 6, 2025 09:31
@nuria1110 nuria1110 requested review from a team as code owners March 6, 2025 09:31
@nuria1110 nuria1110 marked this pull request as draft March 6, 2025 09:35
Fixes issue where margin-bottom applied to Textbox based components would not be applied to
character count.
@nuria1110 nuria1110 marked this pull request as ready for review March 6, 2025 12:05
The use of form spacing context to apply spacing to our inputs within Form has caused issues for
consumers using it in exported components in different projects using Carbon. This fix replaces
context with a pure css solution to ensure margins are applied and consistent across projects.

fix #7058
@nuria1110 nuria1110 marked this pull request as draft March 6, 2025 12:58
@Parsium Parsium self-requested a review March 6, 2025 14:57

When `fieldSpacing` prop is given a value, the vertical spacing between the form fields is changed (margin bottom on each field), the value of the prop multiplied by the base theme `8px` spacing value. The default is value `3` which is therefore `24px`. This can be overriden on a specific field if needed.
You can use the `fieldSpacing` prop to set the spacing between fields within `Form`, which will add a margin-bottom to supported input components.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: this is much clearer to read!

@nuria1110 nuria1110 marked this pull request as ready for review March 12, 2025 09:47
@damienrobson-sage damienrobson-sage self-requested a review March 12, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Form in exported component : wrong margins
3 participants