+
Skip to content

Conversation

clintcs
Copy link
Collaborator

@clintcs clintcs commented Oct 9, 2025

🚀 Description

Minor

Our Form Controls Layout component has been removed. To replace Form Controls Layout, a split attribute has been added to Checkbox, Checkbox Group, Dropdown, Input, Radio Group, Slider, and Textarea.

The value of each component's split attribute matches that of Form Control Layout: split can be 'left", 'middle', 'right', or undefined. You'll need to set split on each component in your form—which isn't as convenient. But it does allow form controls to be laid out independent of Form Controls Layout and of each other if desired.

The impetus for these changes is to prepare for Dropdown moving out of Glide Core:

For components to participate in Form Controls Layout, they previously exposed a privateSplit property that Form Controls Layout set. When Dropdown is moved out of Glide Core, its contract with Form Controls Layout will no longer be guaranteed.

So Dropdown won't be able to participate in Form Controls Layout, and Form Controls Layout would work with every Glide form control component except Dropdown—leaving Dropdown consumers to manually set split on Dropdown.

Rather than oddly have some components that work with Form Controls Layout and some that don't, we decided to remove Form Controls Layout and instead expose split on each component.

I'll wait to merge until @ynotdraw gets back. He'll be interested in reviewing. He may also want to wait to merge until our other unreleased changes have been digested by consuming applications.

📋 Checklist

  • I have followed the Contributing Guidelines.
  • I have added tests to cover new or updated functionality.
  • I have added or updated Storybook stories.
  • I have localized new strings.
  • I have followed the ARIA Authoring Practices Guide or met with the Accessibility Team.
  • I have included a changeset.
  • I have scheduled a design review.
  • I have reviewed the Storybook and Visual Test Report links below.

🔬 Manual Testing

  1. Check out this branch.
  2. Select the "visual" project in Playwright's UI and filter by "split=".
  3. Run all the filtered visual tests.
  4. Spotcheck a handful of test screenshots.
  5. Open Storybook.
  6. Spotcheck a few stories.

Copy link

changeset-bot bot commented Oct 9, 2025

🦋 Changeset detected

Latest commit: 8723339

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crowdstrike/glide-core Minor

Not sure what this means? Click here to learn what changesets are.

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

@clintcs clintcs force-pushed the remove-form-controls-layout branch 2 times, most recently from 5b95902 to 12c8d84 Compare October 9, 2025 16:13
readonly = false;

@property({ attribute: 'select-all', reflect: true, type: Boolean })
selectAll = false;
Copy link
Collaborator Author

@clintcs clintcs Oct 9, 2025

Choose a reason for hiding this comment

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

Alphabetized this and the other properties. No changes other than the addition of split. Same for Slider.

@clintcs clintcs force-pushed the remove-form-controls-layout branch from 12c8d84 to 95086ce Compare October 9, 2025 16:19
@clintcs clintcs marked this pull request as ready for review October 9, 2025 16:20
src/checkbox.ts Outdated
}

set split(split: 'left' | 'middle' | 'right' | undefined) {
if (this.orientation === 'vertical') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this be overkill?

Suggested change
if (this.orientation === 'vertical') {
if (split && this.orientation === 'vertical') {

In case somehow this split setter validly hits with it being set to undefined

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not to me. That seems right given consumers can split and unsplit. I'll ignore this suggestion and push a commit changing it everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

8723339

@clintcs clintcs force-pushed the remove-form-controls-layout branch from 0e1004d to 19d479a Compare October 10, 2025 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载