Skip to content
This repository has been archived by the owner on Jul 24, 2018. It is now read-only.

search product name regardless of letter case to improve user experience #6

Merged

Conversation

Atlas7
Copy link
Contributor

@Atlas7 Atlas7 commented Apr 26, 2016

Benefits:

  • searching for "base" or "Base" or "bAse" etc, will display the product row for "Baseball".
  • improve user experience.
  • very simple change - just a one-liner.

What has changed:

  • add toLowerCase() during the search bar comparison part.

@code0wl
Copy link
Owner

code0wl commented Apr 28, 2016

Hi Atlas,

Thanks for the PR. Can you update this PR so I can merge? :)

@Atlas7
Copy link
Contributor Author

Atlas7 commented Apr 28, 2016

Hey @code0wl, I notice that all the .jsx components have recently been renamed to .js - what is the reason behind this? (I initially thought that by naming the file extension .jsx it gives user / interpreter a clue that the file is in React JSX syntax).

Some references on this debate:

Thanks for creating this repos by the way it has helped me learning some good practice design patterns!

@Atlas7
Copy link
Contributor Author

Atlas7 commented Apr 28, 2016

Just did a git pull and git rebase and git push to this new feature branch. (i.e. new feature now build on top of the most up-to-date master). Hope this is ok!

(would also be grateful if you could shed some light on why changing all the .jsx to .js - to fulfil my curiosity :-)

@code0wl
Copy link
Owner

code0wl commented Apr 28, 2016

@Atlas7 The reasoning behind the renaming is that webpack is handling the resolves for JSX and JS. Not sure which editors need the extension for it to render as JSX. Using Webstorm and Atom at the moment. Both are not complaining. I'll see if more people are having this issue. If so, i'll revert to JSX.

Thanks for your PR :)))). Highly appreciated

@code0wl code0wl merged commit bfaaf4f into code0wl:master Apr 28, 2016
@Atlas7 Atlas7 deleted the feature/search_bar_regardless_of_letter_case branch April 28, 2016 12:39
@Atlas7
Copy link
Contributor Author

Atlas7 commented Apr 28, 2016

Excellent thank you for the merge! (and keep up the great work! :)

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.

2 participants