feat(checkbox, radio, toggle, range): stacked labels for form controls #28021
Conversation
|
|
thetaPC
changed the title
thetaPC
changed the title
github-actions
bot
added
package: angular
| @@ -138,5 +138,31 @@ configs().forEach(({ title, screenshot, config }) => { | |||
| expect(await checkbox.screenshot()).toMatchSnapshot(screenshot(`checkbox-label-fixed-justify-space-between`)); | |||
| }); | |||
| }); | |||
|
|
|||
| test.describe('checkbox: stacked placement', () => { | |||
| test('should render a start alignment with label in the stacked position', async ({ page }) => { | |||
There was a problem hiding this comment.
~ The test descriptions are a bit hard to decipher/read.
Thoughts on rewording to:
should align the label to the start of the container in the stacked position
This would apply to the center align test as well.
| @@ -137,5 +137,35 @@ configs().forEach(({ title, screenshot, config }) => { | |||
| expect(await radio.screenshot()).toMatchSnapshot(screenshot(`radio-label-fixed-justify-space-between`)); | |||
| }); | |||
| }); | |||
|
|
|||
| test.describe('radio: stacked placement', () => { | |||
| test('should render a start alignment with label in the stacked position', async ({ page }) => { | |||
There was a problem hiding this comment.
~ Similar feedback here as checkbox for test descriptions.
| @@ -124,6 +140,22 @@ configs().forEach(({ title, screenshot, config }) => { | |||
|
|
|||
| expect(await range.screenshot()).toMatchSnapshot(screenshot(`range-items-fixed`)); | |||
| }); | |||
| test('should render label in the stacked placement', async ({ page }) => { | |||
There was a problem hiding this comment.
~ Thoughts on rewording to:
should render label above the range slider
| @@ -81,8 +81,9 @@ export class Checkbox implements ComponentInterface { | |||
| * `"start"`: The label will appear to the left of the checkbox in LTR and to the right in RTL. | |||
| * `"end"`: The label will appear to the right of the checkbox in LTR and to the left in RTL. | |||
| * `"fixed"`: The label has the same behavior as `"start"` except it also has a fixed width. Long text will be truncated with ellipses ("..."). | |||
| * `"stacked"`: The label will appear above the checkbox regardless of the direction. | |||
There was a problem hiding this comment.
| * `"stacked"`: The label will appear above the checkbox regardless of the direction. | |
| * `"stacked"`: The label will appear above the checkbox regardless of the direction. The alignment of the label can be controlled with the `align` property. |
There was a problem hiding this comment.
Ditto for the instances of "stacked" for the other components
| align-items: normal; | ||
| } | ||
|
|
||
| :host(.range-label-placement-stacked) .label-text-wrapper { |
There was a problem hiding this comment.
The pin on a range with a stacked label currently overlaps the text. We add padding to the host .range-has-pin is present, but we should probably add that as bottom margin to the label so the pin never obscures the label.
At the very least, you'll need to add bottom margin to the .label-text-wrapper element in each mode stylesheet. You can use the existing padding values that we set on the host when the .range-has-pin class is present in each mode stylesheet.
We then need to remove the host padding since we are setting it as margin on the label wrapper instead. There are a couple ways you can do this:
- Set
@include padding(0px)in the base stylesheet when.range-label-placement-stackedis present. - Update the existing
.range-has-pinselectors to exclude.range-label-placement-stackedusing:not.
We'll also want to capture a screenshot for this.
| /** | ||
| * Label is on top of the checkbox. | ||
| */ | ||
| :host(.checkbox-label-placement-stacked) .checkbox-wrapper { |
There was a problem hiding this comment.
The following applies to checkbox, radio, and toggle (range is not impacted).
When used in an item, there is not enough bottom margin. It looks like we used to account for this on .label-text-wrapper which works fine when the label and control are on the same line, but it looks a little smushed when using stacked layout:
You'll want to apply margin on the top of .label-text-wrapper as well as margin on the bottom of .native-wrapper. So something like this:
:host(.in-item.toggle-label-placement-stacked) .label-text-wrapper {
@include margin($toggle-item-label-margin-top, null, $form-control-label-margin, null);
}
:host(.in-item.toggle-label-placement-stacked) .native-wrapper {
@include margin(null, null, $toggle-item-label-margin-bottom, null);
}We'll want screenshot tests for this too.
Co-authored-by: Sean Perkins <[email protected]>
Co-authored-by: Liam DeBeasi <[email protected]>
Co-authored-by: Liam DeBeasi <[email protected]>
Co-authored-by: Liam DeBeasi <[email protected]>
|
Closing this PR in favor of another PR |
Issue number: resolves #27229
What is the current behavior?
Checkbox, radio, toggle, and range do not allow labels to be stacked.
What is the new behavior?
alignwas added withcenteras the default1label-placementvalue was addedalignwas added withcenteras the default1label-placementvalue was addedalignwas added withcenteras the default1label-placementvalue was addedlabel-placementvalue was addedDoes this introduce a breaking change?
Other information
ion-docsFootnotes
centeris being used becausealign-items="center"was the original style prior to the implementation. Otherwise, the label will not line up correctly on default. For example, the checkbox for iOS will shift the label to the top left corner whenaligndefaulted tostart.2 This shift happens because the input is larger than the label, making the alignment obvious. But this is not correct, the label must be centered vertically as the default to prevent breaking changes to the current visualizations. ↩ ↩2 ↩3