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

Proposed fix for Kudos marketplace search bar #5376

Merged
merged 2 commits into from
Nov 6, 2019

Conversation

0xsama
Copy link
Contributor

@0xsama 0xsama commented Oct 24, 2019

Description

When you add whitespace in search bar value, for example: "Limited edition", the query parameters will be "Limited+edition", so when results come back in the view, the search bar get its value from query parameters (queryParam.q), so it will also show up in the search bar as Limited+edition. The simplest solution is to replace the plus signs with whitespace:\

$('#kudos-search').val(queryParam.q.replace(/+/g, " "));

I think it does look better.

Refers/Fixes
Testing

simplescreenrecorder-2019-10-28_10 27 32

When you add whitespace in search bar value, for example: "Limited edition", the query parameters will be "Limited+edition", so when results come back in the view, the search bar get its value from query parameters (queryParam.q), so it will also show up in the search bar as Limited+edition. The simplest solution is to replace the plus signs with whitespace, i think it does look better.
@danlipert
Copy link
Contributor

@ososco nice! Thanks for the PR! In the PR description, theres a section to add how you tested your changes - here a screenshot or video would be good, or just a brief description on how you verified the changes are working. Thanks again!

@0xsama
Copy link
Contributor Author

0xsama commented Oct 28, 2019

@danlipert

simplescreenrecorder-2019-10-28_10 27 32

@0xsama
Copy link
Contributor Author

0xsama commented Oct 28, 2019

Actually there is a better way to fix this with decodeURIComponent, i will commit the new fix after testing it.

Remove plus sign "+" in URL query string by decoding the query using "decodeURIComponent", and replace plug sign with "%20" before the decode in case the plug sign is part of the search query, such in "C++"
@codecov
Copy link

codecov bot commented Oct 29, 2019

Codecov Report

Merging #5376 into master will increase coverage by 2.55%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5376      +/-   ##
==========================================
+ Coverage   29.67%   32.23%   +2.55%     
==========================================
  Files         242      244       +2     
  Lines       20601    25532    +4931     
  Branches     2968     4348    +1380     
==========================================
+ Hits         6114     8231    +2117     
- Misses      14236    16898    +2662     
- Partials      251      403     +152
Impacted Files Coverage Δ
app/app/urls.py 83.33% <0%> (-6.67%) ⬇️
app/bounty_requests/forms.py 50% <0%> (-3.85%) ⬇️
app/grants/views.py 13.99% <0%> (-2.41%) ⬇️
app/faucet/views.py 26.5% <0%> (-2.07%) ⬇️
app/kudos/views.py 20.96% <0%> (-1.14%) ⬇️
app/dashboard/tip_views.py 17.67% <0%> (-1.05%) ⬇️
app/dashboard/helpers.py 13.02% <0%> (-0.96%) ⬇️
app/kudos/utils.py 19.29% <0%> (-0.86%) ⬇️
app/bounty_requests/views.py 29.62% <0%> (-0.81%) ⬇️
app/retail/views.py 27.29% <0%> (-0.51%) ⬇️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0050d1c...92371e0. Read the comment docs.

Copy link
Contributor

@danlipert danlipert left a comment

Choose a reason for hiding this comment

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

Nice, looks great!

@octavioamu octavioamu changed the base branch from 072519rc to master November 4, 2019 13:50
@octavioamu
Copy link
Contributor

@ososco I changed the base branch to be 'master'

@octavioamu octavioamu merged commit 854756f into gitcoinco:master Nov 6, 2019
@gitcoinbot
Copy link
Member

⚡️ A tip worth 0.20000 ETH (38.05 USD @ $190.27/ETH) has been granted to @ososco for this issue from @owocki. ⚡️

Nice work @ososco! Your tip has automatically been deposited in the ETH address we have on file.

@gitcoinbot
Copy link
Member

Eye For Detail ⚡️ A *Eye For Detail* Kudos has been sent to @ososco for this issue from @owocki. ⚡️

Nice work @ososco!
Your Kudos has automatically been sent in the ETH address we have on file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants