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(many): form controls labels have increased margin #27447

Merged
merged 6 commits into from
May 16, 2023
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented May 10, 2023

Issue number: resolves #27129


What is the current behavior?

Material Design has 16px of margin between the form control and the label, but we have 8px: https://m2.material.io/components/selection-controls#usage

What is the new behavior?

  • Updates default margin from 8px to 16px for checkbox, input, radio, range, select, textarea, and toggle.

Note: This should only apply to labels that are on the same line as the form control. In other words, they do not apply to stacked/floating labels.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@stackblitz
Copy link

stackblitz bot commented May 10, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@liamdebeasi liamdebeasi changed the title Fw 3969 fix(many): form controls labels have increased margin May 10, 2023
@github-actions github-actions bot added the package: core @ionic/core package label May 10, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review May 11, 2023 16:36
@liamdebeasi liamdebeasi requested a review from a team as a code owner May 11, 2023 16:36
@liamdebeasi liamdebeasi marked this pull request as draft May 11, 2023 16:39
}

::slotted([slot="end"]) {
@include margin(0, 0, 0, var(--margin));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I incorrectly assumed that the margins in the slots should be the same as the margin between the control and the label, so I reverted the --margin usage.

@liamdebeasi liamdebeasi marked this pull request as ready for review May 11, 2023 18:59
Copy link
Contributor

@averyjohnston averyjohnston left a comment

Choose a reason for hiding this comment

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

Full disclosure that I didn't look through all the screenshots; Github seems to have somehow gotten even worse at rendering large PRs like this 😅 In the future it may be useful to make the changes per component and run the screenshot job each time, so that the screenshot commits are broken up into smaller chunks.

@liamdebeasi liamdebeasi added this pull request to the merge queue May 15, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 15, 2023
@liamdebeasi liamdebeasi added this pull request to the merge queue May 15, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 15, 2023
@liamdebeasi liamdebeasi added this pull request to the merge queue May 16, 2023
Merged via the queue into main with commit 381de0b May 16, 2023
@liamdebeasi liamdebeasi deleted the FW-3969 branch May 16, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: form controls do not have enough margin
3 participants