search

keboola / indigo-ui

Indigo UI library, its CSS and React components. Also Styleguide app showing usages of this library.
https://indigo.keboola.com
MIT License
5 stars 0 forks source link

Clearable Searchbar #301

Closed janmichek closed 5 years ago

janmichek commented 5 years ago

fixes #300

janmichek commented 5 years ago

Jeste se snazim udelat ten focus po clearu

janmichek commented 5 years ago

Eslinteru se nelibi pouziti refs, tak na to musim prijit jinak

natocTo commented 5 years ago

Můžeš využít ten inputRef tam.

Něco jako:

inputRef={(input) => {
  this.searchbar = input;

  if (this.props.inputRef) {
    this.props.inputRef(input);
  }
}}

a pak už by to mělo být ok použít.. 

handleClear(event) {
  event.preventDefault();
  this.setState({ query: '' });
  this.props.onClear();
  this.searchbar.focus();
}

Eslintu se asi nelíbí pouze string refs. To je deprecated a používá se ta funkce, nebo v novém rectu je ještě nějaký další způsob.

janmichek commented 5 years ago

Dobry, diky za tip @natocTo chvili jsem zkoumal jak to teda funguje. Zkusil jsem jestli to nahodou nerozbilo 'Callback Refs' demo a je to ok, i kdyz jsem k demu pridal clearable.

natocTo commented 5 years ago

OK. tu je to ready na review?

janmichek commented 5 years ago

jj, prosim

natocTo commented 5 years ago

Kdyby to byla úplně nová komponenta, tak bych skoro řekl že každý searchbar by mohl být clearable. Jakože ta props by nebyla potřeba. Ale takto když už to je používáno je to asi lepší takto.

janmichek commented 5 years ago

Možná bych tam ten křížek (clear button) dal až když něco napíši

Jo, supr postreh

Kdyby to byla úplně nová komponenta, tak bych skoro řekl že každý searchbar by mohl být clearable.

Taky jsem se nad tim zamyslel - vetsinou to budeme chtit. Nenapada me duvod proc bychom to clear nechteli - Mozna jen uzky searchbar? Nevim. Provizorne tu props teda vyhodim. Kdyby se pri implementaci ukazalo ze to chybi, muzeme to snadno revertnout.

janmichek commented 5 years ago

@natocTo Fixed, jeste jsem teda vyhodel ten outline styl na focus. Moc se mi to tam nelibilo

natocTo commented 5 years ago

Good, ještě další věc.

Pokud by se teď ta komponenta vydala a poslala do KBC-UI, tak ten křížek sice vymaže zadaný text, ale tím to končí. Koukal jsem na konkrétní použití tam a skoro všude je implementováno pouze onChange (dokonce onKeyDown není snad nikde, ale to je jedno). A aby zjistili, že to bylo vymazáno, tak by všechny SearchBary musely implementovat onClear.

Myslím že by se onClear mohlo úplně vyhodit. Nebo nevím jaký to má use case. Místo toho by se v handleClear mohlo volat onChange('')

ujovlado commented 5 years ago

V devportali to je spravene presne ako pises: https://github.com/keboola/developer-portal-ui/blob/0de6f2a4a0725ce66b3ee763e9aee4ce4daff755/src/scripts/common/form/SearchInput.js#L32

janmichek commented 5 years ago

Ok, jo to mi asi nedoslo, fixnu to. 'onKeyDown' se pouziva ve vyberu projektu po prihlaseni, nebo v sidebaru. Muzes si sipkama projizdet seznam

natocTo commented 5 years ago

Jo díky za doplnění. :)

janmichek commented 5 years ago

Jo, fixnul jsem to. Chystal jsem se to vyzkouset v kbc v tomto kole. Jeste jsem doupravil 2 styly.

Jeste tam bude jedna implementacni vec: Napriklad v seznam Jobu nebo Logu se vyhledava az po odpaleni query entrem - ostatni searche vyhledavaji 'live'. Tak to bude treba napojit.

ujovlado commented 5 years ago

Nezhorsi vyhodenie outline ovladanie klavesnicou? Nebudem vediet, kde som. https://a11yproject.com/posts/never-remove-css-outlines/

ujovlado commented 5 years ago

@natocTo az to bude z tvojej strany ok, daj vediet, dam tomu este rychly check a mergol by som to.

janmichek commented 5 years ago

Feer point, jeden z tech stylu jsem vratil. Zvyrazni to button pokud tam 'doTabujes', ale na klik ne nedela outline

natocTo commented 5 years ago

Ve stories zůstalo ještě info o onClear ale to je detail. Plus stejný detail je asi to this.state.query.length > 0 kde to ve mě trochu evokuje co je to query teda zač, u stringu bych preferoval kratší zápis this.state.query ale to není nic zásadního. Jinak myslím ok tak to potvrdím.

janmichek commented 5 years ago

Vytvorim teda jeste issue na implementaci clearovani searchu submitovanych enterem.

janmichek commented 5 years ago

ok, jeste vysmahnu ten onClear z dema

ujovlado commented 5 years ago

Vytvorim teda jeste issue na implementaci clearovani searchu submitovanych enterem.

tomu moc nerozumiem, co presne tam treba zmenit?

janmichek commented 5 years ago

Tady https://github.com/keboola/indigo-ui/pull/301#issuecomment-433045651 myslim ze je to vec implementace. Napr si zkus clear v Jobech. Query sice zmizi, ale search se nevyresetuje

janmichek commented 5 years ago

Uzivatelsky by mi prislo dobry aby to delalo oboje, nebo?

janmichek commented 5 years ago

Urcite to souvisi is tou issue v Recommended searches

ujovlado commented 5 years ago

Aha, chapem ... takze clear by zavolal vlastne change s prazdnou hodnottou a ak existuje submitFn, tak aj to ... jo, to myslim dava zmysel

janmichek commented 5 years ago

https://github.com/keboola/kbc-ui/issues/2228

ujovlado commented 5 years ago

screenshot_2018-10-25_21-43-54