feat(input): add start and end slots #28397

averyjohnston · 2023-10-23T18:03:25Z

Issue number: Part of #26297

What is the current behavior?

With the modern form control syntax, it is not possible to add icon buttons or other decorators to the sides of ion-input, as you can with ion-item.

What is the new behavior?

start and end slots added. While making this change, I also tweaked the CSS selectors responsible for translating the label above the input with "stacked" and "floating" placements, merging this logic into a single class managed by the TSX. I needed to add a new class for whether slot content is present, so the selectors were getting unwieldy otherwise.

Docs PR TBA; I plan on knocking out all three components at once when the features are all complete, to make dev builds easier to manage.

Does this introduce a breaking change?

Yes
No

Other information

… slot

averyjohnston · 2023-10-23T18:22:48Z

core/src/components/input/input.tsx

-          'has-focus': this.hasFocus,
+          'has-value': hasValue,
+          'has-focus': hasFocus,
+          'label-floating': labelShouldFloat,


Open to alternative ideas for the name of this class.

I think this name makes sense. We tend to do <component>-<status> when naming them. Like the following:

select-disabled select-expanded radio-checked radio-disabled

If you really wanted to you could add input before it since we have the following in other components (radio, select) when talking about the label placement:

[`radio-label-placement-${labelPlacement}`]: true,

core/src/components/input/input.scss

averyjohnston · 2023-10-23T18:39:53Z

core/src/components/input/input.tsx

+    const hasStartEndSlots = el.querySelector('[slot="start"], [slot="end"]') !== null;
+
+    /**
+     * If the label is stacked, it should always sit above the input.
+     * For floating labels, the label should move above the input if
+     * the input has a value, is focused, or has anything in either
+     * the start or end slot.
+     *
+     * If there is content in the start slot, the label would overlap
+     * it if not forced to float. This is also applied to the end slot
+     * because with the default or solid fills, the input is not
+     * vertically centered in the container, but the label is. This
+     * causes the slots and label to appear vertically offset from each
+     * other when the label isn't floating above the input. This doesn't
+     * apply to the outline fill, but this was not accounted for to keep
+     * things consistent.
+     */
+    const labelShouldFloat =
+      labelPlacement === 'stacked' || (labelPlacement === 'floating' && (hasValue || hasFocus || hasStartEndSlots));


Auto-floating when there's anything in the start slot is taken from MUI: https://stackblitz.com/edit/react-7vghat?file=Demo.js

The end slot behavior was a compromise and does not follow native behavior. I spent a bit of time trying to rearrange the CSS/markup to vertically center the input when the label isn't raised, without breaking existing behavior, but in the end decided it would be best to push that scope to a future ticket since the feature works as-is and there are still two components to go.

Let me know if you feel that allowing the label to lower with content in the end slot should be required for this feature, and I can take a closer look. Alternatively, I can allow that only for fill="outline" for now and fix up the other fills in a separate ticket.

Material Design components for the web seem to have the floating label overlay the input even when using a start slot: https://material-components.github.io/material-components-web-catalog/#/component/text-field However, it sounds like that would significantly increase the amount of work required to implement. Given that this is an experimental feature, I'm fine going without this behavior.

The MUI demo looks like it only floats the label when there is a start slot but not when there is only an end slot. Should we match that? Or is that what you meant was a compromise?

Also, just adding that it seems the latest Material Design components for the web also overlay the input with a start slot: https://material-web.dev/components/text-field/#icons

(I modified the first md-outlined-field to include label="Messages":

floating-labels-slots.mov

Their implementation doesn't look complete though judging by the zero padding between the icons and edge of the input lol. I'm also okay with not implementing this, but maybe we should make a tech debt ticket to look into it more?

The MUI demo looks like it only floats the label when there is a start slot but not when there is only an end slot. Should we match that? Or is that what you meant was a compromise?

That's what I meant, yeah. I can definitely make a tech debt ticket if we're in agreement, I just wanted to make sure we were okay proceeding without this piece for now.

Tech debt ticket created: https://ionic-cloud.atlassian.net/browse/FW-5592

liamdebeasi

I needed to pause my review, but wanted to get these comments out now so you can start addressing them. Overall the implementation looks solid! Mostly nitpick things

core/src/components/input/input.scss

liamdebeasi · 2023-10-26T15:46:52Z

core/src/components/input/input.tsx

+    const hasStartEndSlots = el.querySelector('[slot="start"], [slot="end"]') !== null;
+
+    /**
+     * If the label is stacked, it should always sit above the input.
+     * For floating labels, the label should move above the input if
+     * the input has a value, is focused, or has anything in either
+     * the start or end slot.
+     *
+     * If there is content in the start slot, the label would overlap
+     * it if not forced to float. This is also applied to the end slot
+     * because with the default or solid fills, the input is not
+     * vertically centered in the container, but the label is. This
+     * causes the slots and label to appear vertically offset from each
+     * other when the label isn't floating above the input. This doesn't
+     * apply to the outline fill, but this was not accounted for to keep
+     * things consistent.
+     */
+    const labelShouldFloat =
+      labelPlacement === 'stacked' || (labelPlacement === 'floating' && (hasValue || hasFocus || hasStartEndSlots));


Material Design components for the web seem to have the floating label overlay the input even when using a start slot: https://material-components.github.io/material-components-web-catalog/#/component/text-field However, it sounds like that would significantly increase the amount of work required to implement. Given that this is an experimental feature, I'm fine going without this behavior.

liamdebeasi · 2023-10-26T15:59:49Z

core/src/components/input/input.scss

+// ----------------------------------------------------------------
+
+::slotted([slot="start"]) {
+  margin-inline-end: $form-control-label-margin;


The focus state looks a bit odd on the button:

Maybe we need to add some padding?

Oh actually it looks like the padding was removed in the test template. Any particular reason for this?

I just thought it looked nicer 😆 With the default padding added to the margins, it looks like there's a ton of empty space around the icons. I'll revert the padding so the behavior is more clear.

Oh I see why you added it 😂

Yeah that does look strange. We might want to consider adding some internal styles to account for this. cc @brandyscarney might have ideas on how best to achieve this. I'd expect the start button to line up with the stacked label at the very least.

I looked into this a bit and is there a reason we are recommending buttons for both slots? In the Material Design web catalog they use plain icons. The latest Material Design web components does include a button in the end slot for their "Password" field, although their padding is nonexistent, but I suspect it just hasn't been properly designed yet.

Maybe we should add some styles for slotted icons to make them appear larger and match? Here is what I see with an ion-icon in the start slot and an ion-button in the end slot:

In addition, maybe we could style the .button-has-icon-only like this:

:host(.button-has-icon-only) .button-native { @include padding(0); aspect-ratio: 1; border-radius: 50%; }

Changing the icon in the start slot back to a button, with the above styles, gives me:

This still looks a bit off because of the left padding, but if we reduce that it also moves the notch. We could remove the left margin for button and get it a bit closer:

I think we should focus more on aligning icons in the start slot with the label though. It doesn't seem to be a typical pattern to have a button slotted at the start. Thoughts?

That's a good point that a button in the start slot isn't going to be a common use case. I didn't make the test page with recommended patterns in mind -- the buttons were mostly to check the focus behavior -- but I agree that the icons should be the priority, at least as far as the start slot goes.

That said, I'm hesitant to add too much built-in styling since I don't want devs to have to deal with potentially unwanted extra "magic," especially since they can add their own styling to the slotted content. For example, if we removed the padding from icon-only buttons by default, that would make non-clear buttons look odd:

Maybe we just adjust the test to make it seem less like I'm trying to recommend using clear buttons in both slots?

It should still look fine with the CSS I added above:

but I understand if we don't want to add custom styles just for this. I do think we should consider it in the future though, because Material Design 3 has a section dedicated to icon buttons that looks like the above: https://m3.material.io/components/icon-buttons/overview

Maybe this is something we could add for our Material Design 3 work.

Their spec doesn't show buttons in an input at all so yeah I am fine with pushing this off: https://m3.material.io/components/text-fields/specs

We can also push off styling icons in an input but I think we should revisit that for MD3 too.

We decided to handle this as part of a separate ticket, starting with this spike: https://ionic-cloud.atlassian.net/browse/FW-5645

core/src/components/input/input.tsx

liamdebeasi

Overall looks good! I have one lingering comment about the slotted button padding, but I think the general approach makes sense.

Devolick · 2023-10-31T17:06:42Z

Hi, approximately how long until this will be available? Is it possible to add a collection from *ngFor of via slot=start to make multiselect with floating label and popover?

Best,
Dzmitry

averyjohnston · 2023-10-31T17:38:33Z

@Devolick We do not have a planned timeline for this feature; work is ongoing and there are other components that need updating as well before this can be shipped.

For your use case, I would instead recommend using a Select, or a Popover that opens underneath the input according to your desired behavior. For questions on the implementation, I would recommend posting in the Ionic forum or Discord server.

brandyscarney

This looks good I just added a couple comments about the naming and button/icon styles!

amandaesmith3 added 5 commits October 20, 2023 10:51

update slotMutationController to allow multiple slot names

d2bb3e4

add slots and tests

1792644

always float label if slot content is present

47163b5

make clicking label focus input even if there's a button in the start…

affd46e

… slot

add comments to labelShouldFloat

ac2be10

github-actions bot added the package: core @ionic/core package label Oct 23, 2023

amandaesmith3 and others added 4 commits October 23, 2023 13:22

lint

ba03215

add slotted buttons to a11y test

02bd324

add e2e tests

dc814d3

chore(): add updated snapshots

2401b70

averyjohnston marked this pull request as ready for review October 23, 2023 19:41

averyjohnston requested review from a team and brandyscarney and removed request for a team October 23, 2023 19:42

averyjohnston commented Oct 23, 2023

View reviewed changes

liamdebeasi suggested changes Oct 26, 2023

View reviewed changes

amandaesmith3 added 2 commits October 26, 2023 14:30

more comments

91721a5

revert padding around buttons in test HTML

ea352dd

averyjohnston requested a review from liamdebeasi October 26, 2023 19:53

liamdebeasi approved these changes Oct 30, 2023

View reviewed changes

brandyscarney approved these changes Nov 1, 2023

View reviewed changes

brandyscarney mentioned this pull request Nov 7, 2023

ion-icons not being aligned correctly in label and button #18033

Closed

amandaesmith3 added 2 commits November 14, 2023 13:17

add tech debt ticket

4381cb5

lint

62cd6f0

Base automatically changed from FW-2853-base to FW-2853 November 27, 2023 15:44

averyjohnston merged commit 3648520 into FW-2853 Nov 27, 2023

averyjohnston deleted the FW-2853-input branch November 27, 2023 15:47

feat(input): add start and end slots #28397

feat(input): add start and end slots #28397

Uh oh!

Conversation

averyjohnston commented Oct 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Other information

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandyscarney Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

averyjohnston Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liamdebeasi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

liamdebeasi left a comment

Choose a reason for hiding this comment

Uh oh!

Devolick commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

averyjohnston commented Oct 31, 2023

Uh oh!

brandyscarney left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

averyjohnston commented Oct 23, 2023 •

edited

Loading

brandyscarney Nov 1, 2023 •

edited

Loading

averyjohnston Oct 27, 2023 •

edited

Loading

Devolick commented Oct 31, 2023 •

edited

Loading