search

cozy / cozy-contacts-v2

Contact books manager for Cozy
http://cozy.io
GNU Affero General Public License v3.0
20 stars 20 forks source link

Fix contacts filtering when label contains space #288

Closed gregorylegarec closed 7 years ago

gregorylegarec commented 7 years ago

Fix #262

When a contact label is clicked, it is used as an URL segment to trigger a search.

Also, the label's name is used as a class associated to the HTML element containing contacts info. The mechanism used is to use a CSS hiding to render the filtering (in my opinion we should not rely on DOM logic to handle data logic like filtering, this issue is an evidence).

If the new CSS rule created when a filtering is performed contains a space character, it will be malformed an trigger an error. This error blocks the whole filtering flow.

Fix :

The first idea was to generate slugs server-side or when server data is parsed client-side. But this solution implies to replace actuals tags (which are strings) by objects with who properties {name, slug}, and it impacts the whole application. As the risk of regression was too high, I decided to transform the name into a slug at the last moment, which is the safest solution but absolutely not the better in term of design and calculation time.

Thanks @m4dz to have a review.

gregorylegarec commented 7 years ago

Actually this PR seems to solve all character related problem like https://github.com/cozy/cozy-contacts/pull/286, as it slugs the whole tag name.

m4dz commented 7 years ago

Great! Thanks

we should not rely on DOM logic to handle data logic like filtering

I don't totally agree because of poor JS perfs when we need to re-render a whole list of bunch contacts, when it's just matter of presentation (which rely on CSS), such as hiding/showing elements

generate slugs server-side

I'm OK with this, but it relies on the tags mechanics on the stack IMHO.

gregorylegarec commented 7 years ago

Thanks :)

I don't totally agree because of poor JS perfs when we need to re-render a whole list of bunch contacts, when it's just matter of presentation (which rely on CSS), such as hiding/showing elements

What bother me is that there is no list anywhere containing the current state of filtered elements, and we are totally trusting the DOM for doing the job. Also, your argument about performance is understandable.

I'm OK with this, but it relies on the tags mechanics on the stack IMHO.

Yep, I feel that the tag mechanism is under-designed and hope that we will take on opportunity to improve or change it with the new stack :)

m4dz commented 7 years ago

Yep, I feel that the tag mechanism is under-designed and hope that we will take on opportunity to improve or change it with the new stack :)

/poke @nono :smile: