svelte-materialify icon indicating copy to clipboard operation
svelte-materialify copied to clipboard

TextField label has some bugs / edge cases

Open atsepkov opened this issue 4 years ago • 6 comments

This is actually a few separate smaller bugs / UI inconsistencies I noticed. Try rendering the following element:

<TextField outlined filled placeholder="Search" />

This is a screenshot: searchbox

Problems observed (seems to be due to interaction of 'filled' attribute with others, and perhaps outlined as well):

  1. Omitting TextField label results in an ugly white box rendering (hiding the label altogether here would be better).
  2. Even with text present in the label, the box background should be transparent, not white.
  3. There is a lot of inconsistency to the height of the box between different rendering modes (outlined seems to add at least 8-10 pixels to the height and filled seems to add a few more, dense seems to subtract some but the interaction with other attributes is not intuitive). I would prefer (and I think most users would as well) if outline and filled did not affect padding, margin or height of the element at all, and that either dense or normal modes would align to standard button height. Otherwise placing text box and a button next to each other (as is often done in UI) looks amateurish on a page.

atsepkov avatar Feb 25 '21 03:02 atsepkov

@TheComputerM I think the API name placeholder is misleading, we should call it label (or overwriteLabel) @atsepkov <TextField outlined filled>Search</TextField> works, basically it expects tags to be filled. And yeah, we should code around that.

Florian-Schoenherr avatar Feb 25 '21 12:02 Florian-Schoenherr

@Florian-Schoenherr based on my testing, placeholder seems to work as expected (it gets replaced as soon as the text is typed). But I do agree that label should be an attribute rather than a text node child of the TextField. That is inconsistent with the convention many other UI libraries adopted (where label and value are flipped). I recommend making label an optional attribute, and value be the text content inside the TextField.

atsepkov avatar Feb 25 '21 12:02 atsepkov

You mean the slot content should be the placeholder? value is what the user has written into the field.

Florian-Schoenherr avatar Feb 25 '21 13:02 Florian-Schoenherr

No, I think placeholder is fine the way it is. Regarding value, there are many cases when developer would want to pre-populate the value of a text field (even if the user overwrites it later). For example, when end user fetches their own user profile, the fields are typically pre-populated with existing data but allow user to update it. Actually looking at a couple React libs (because there are more examples to choose from in React than Svelte and the syntax is similar enough), value also seems to be a common attribute for pre-populated text.

Here is a random React library I just googled as an example : https://material-ui.com/components/text-fields/

Based on that, I would actually recommend not using nested text in TextField at all, moving what is currently nested text into label like so:

// before (example)
<TextField class="float-left mr-2" outlined dense filled>
    <div slot="append">
        <Icon path={mdiMagnify} />
    </div>
    Search
</TextField>

// after
<TextField class="float-left mr-2" outlined dense filled label="Search">
    <div slot="append">
        <Icon path={mdiMagnify} />
    </div>
</TextField>

But that's just a convention and is secondary. The main problem I'm seeing right now is that TextField doesn't seem to be designed to properly handle blank/no label (at least combined with outlined and filled attributes). And then there is an arbitrary margin that seems to be present at the top, even when using dense style.

atsepkov avatar Feb 25 '21 14:02 atsepkov

I actually had the same issue, I spent a bit of time tinkering but not enough to commit.

My understanding is that it should be as follows:

<Textarea bind:value={text}>My Label <Textarea>

The above goes inside of

// Textarea.svelte
...
<div class="s-text-field__input">
      <label for={id} class:active={labelActive}>
        <slot /> // <- My Label enters here ?
      </label>
      <textarea
...

To my understanding, labelActive should be false if if the default value is empty/null <Textarea bind:value={text}> <Textarea> -> labelActive = false;

Alternatively, we have an export let label = true as default, and the user declares label={false} inside of component declaration?

dmitrioligy avatar Feb 25 '21 21:02 dmitrioligy

Note: <TextField placeholder="Search">Something else</TextField> ^this is also allowed

Florian-Schoenherr avatar Feb 26 '21 08:02 Florian-Schoenherr