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

Input: remove need for icon to support 'loading' prop #1716

Closed
TimNZ opened this issue May 30, 2017 · 4 comments
Closed

Input: remove need for icon to support 'loading' prop #1716

TimNZ opened this issue May 30, 2017 · 4 comments

Comments

@TimNZ
Copy link

TimNZ commented May 30, 2017

Allow 'loading' prop to work on Input without the need for an 'icon' to be specified.

I'm currently using icon=" " which throws a validation error, still works though.

@layershifter
Copy link
Member

layershifter commented May 30, 2017

I'm not sure that it's really need to be implimented, here is description of a loading example:

An icon input field can show that it is currently loading data.

Here is example html output of your workaround:

<Input loading icon=' ' placeholder='Search...' />
<div class="ui loading icon input">
  <input type="text" placeholder="Search..." />
  <i aria-hidden="true" class="  icon"></i>
</div>

It renders an empty i element, it's required markup by SUI, BTW. So, I'm not sure that it's a correct use case, however @levithomason can have an another opinion.

@TimNZ
Copy link
Author

TimNZ commented May 30, 2017

I agree with what you are saying now I read the documentation more thoroughly.

The question is why whoever originally decided Loading only needed to apply to icons.

I'll look at alternatives.
Thanks.

@TimNZ TimNZ closed this as completed May 30, 2017
@levithomason
Copy link
Member

Per the loading docs, an icon is required in order to show the loading state:

Loading inputs automatically modify the input's icon on loading state to show loading indication.

This seems silly, I'd expect the loading state to work just as error and success states work when used alone. I'd gladly take a PR that adds this.

Proposal

If no icon is specified but loading is, we can simply render any icon, perhaps a spinner.

<Input loading placeholder='Search...' />
<div class="ui loading icon input">
  <input type="text" placeholder="Search..." />
  <i aria-hidden="true" class="spinner icon"></i>
</div>

@TimNZ
Copy link
Author

TimNZ commented Jun 1, 2017

Great. I was going to work on a PR but since I've committed to MVP critical path things for a launch didn't need the distraction and had a work-around.

Plus I would have to spend time understanding the src structure, you fancy coders confuse me :)

In the calendar to do next week.

@layershifter layershifter changed the title Remove need for icon to support 'loading' prop on Input Input: remove need for icon to support 'loading' prop Jun 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants