Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(input): better mobile experience #93

Merged
merged 9 commits into from
Apr 13, 2017
Merged

Conversation

rayrutjes
Copy link
Member

According to the wonderful reading about search UX ;)

Magnifying is a theming concern.

Not sure about submit button though, nowadays, no JS is very rare.

@rayrutjes rayrutjes requested a review from Haroenv April 12, 2017 12:00
v-model="query"
ref="input"
:placeholder="placeholder"
>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm missing the reset button here.

I understand that there's no submit button by default, but a reset is a feature that only a few browsers have, so ...

Copy link
Member Author

@rayrutjes rayrutjes Apr 12, 2017

Choose a reason for hiding this comment

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

I agree but I would like to keep as much granularity as possible regarding components.
I'm already not very happy with the input no longer being the root element of this component because I now have to forward placeholder.

Maybe we should introduce a component like SearchForm:

<ais-form>
    <ais-input/>
    <ais-clear/>
    <ais-submit/>
</ais-form>

Maybe I should just add the clear to the input component, but then it shouldn't be called input anymore.

v-model="query"
>
<form role="search" action="" @submit.prevent="onFormSubmit">
<input type="search"
Copy link
Contributor

Choose a reason for hiding this comment

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

submit button and its "submit on enter" behaviour is missing here. Is that on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

For me the submit works without that button so I didn't add it.

Copy link
Member Author

Choose a reason for hiding this comment

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

On what kind of browser doesn't it submit on keypress if now submit button is present?

Copy link
Contributor

Choose a reason for hiding this comment

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

People that don't press enter, but click.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, but given we don't add styles by default, it will mean that users will need to hide this button everytime. Not an ideal workflow. Maybe we can leverage v-show here.

@rayrutjes
Copy link
Member Author

I will tackle this tomorrow to come up with a solution that is more satisfying than the current implementation. I agree that providing the reset button would be better. Maybe changing the naming to reflect that could help.

Current is ais-input but tomorrow it will be a form + input + reset + submit.

PS: thanks to your suggestions I learned that you can natively clear a search input with ESC even without reset button ;)

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

Looks good now!

<ais-input :placeholder="placeholder"></ais-input>
<button type="submit" :class="bem('submit')">
<svg xmlns="http://www.w3.org/2000/svg" width="1em" height="1em" viewBox="0 0 40 40">
<path
Copy link
Contributor

@Haroenv Haroenv Apr 13, 2017

Choose a reason for hiding this comment

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

can you add a <title> to this with search, which is translatable?

</button>
<ais-clear>
<svg xmlns="http://www.w3.org/2000/svg" width="1em" height="1em" viewBox="0 0 20 20">
<path
Copy link
Contributor

@Haroenv Haroenv Apr 13, 2017

Choose a reason for hiding this comment

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

here as well, with clear

@rayrutjes
Copy link
Member Author

Regarding translations, it is a concept we will need to discuss at some point I guess. For now, I just exposed the 2 titles as props.

@rayrutjes rayrutjes merged commit 0fb0fd8 into master Apr 13, 2017
@rayrutjes rayrutjes deleted the fix/search-input-mobile branch April 13, 2017 13:03
Haroenv pushed a commit to algolia/instantsearch that referenced this pull request Dec 28, 2022
…ch-input-mobile

fix(input): better mobile experience
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants