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

fix: Only show country dropdown without autocomplete #5652

Merged
merged 25 commits into from
Nov 20, 2020
Merged

fix: Only show country dropdown without autocomplete #5652

merged 25 commits into from
Nov 20, 2020

Conversation

divyamtayal
Copy link
Member

@divyamtayal divyamtayal commented Nov 15, 2020

Fixes #5438

Short description of what this resolves:

The problem was that all the countries which are defined in demography were showing. Now showing only payment countries

Changes proposed in this pull request:

  • Instead of showing all countries showing payment countries.

Before

Screenshot from 2020-11-16 04-38-46

After

Screenshot from 2020-11-16 04-37-54

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Nov 15, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/nibnrl3rq
✅ Preview: https://open-event-frontend-git-only-countries-5438.eventyay.now.sh

@divyamtayal divyamtayal changed the title Show Payment countries only Only show country dropdown, do not automatically fill in random info - only country Nov 15, 2020
@codecov
Copy link

codecov bot commented Nov 15, 2020

Codecov Report

Merging #5652 (6f25411) into development (7d00804) will decrease coverage by 0.15%.
The diff coverage is 50.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5652      +/-   ##
===============================================
- Coverage        23.73%   23.57%   -0.16%     
===============================================
  Files              509      510       +1     
  Lines             5415     5417       +2     
  Branches            59       59              
===============================================
- Hits              1285     1277       -8     
- Misses            4114     4124      +10     
  Partials            16       16              
Impacted Files Coverage Δ
app/components/country-dropdown.ts 50.00% <50.00%> (ø)
app/components/smart-overflow.js 61.53% <0.00%> (-38.47%) ⬇️
app/components/modals/tax-info-modal.js 11.11% <0.00%> (-11.12%) ⬇️
app/controllers/index.js 27.27% <0.00%> (-9.10%) ⬇️
app/components/forms/user-payment-info-form.js 20.00% <0.00%> (-6.67%) ⬇️

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 7d00804...20cdc1a. Read the comment docs.

@divyamtayal
Copy link
Member Author

@iamareebjamal Please see the changes I have made.

@@ -96,6 +97,10 @@ export default class UserPaymentInfoForm extends Component.extend(FormMixin) {
return orderBy(countries, 'name');
}

get paymentCountries() {
return orderBy(filter(countries, country => paymentCountries.includes(country.code)), 'name');
Copy link
Member

Choose a reason for hiding this comment

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

Use js native filter

@iamareebjamal
Copy link
Member

This does not solve the issue. Read the issue again

@divyamtayal
Copy link
Member Author

@iamareebjamal the issue what I think is that while for eg searching for India they are lots of countries coming, so do I have to not show other unnecessary countries in drop down o and not change anything with no of countries that are already there in it.

Bcz like for eg in wizard step 1 during choosing country there are only 44 countries in the dropdown but here there are 200+ so if we search India for eg here it will be showing more no. of countries bcz here in UiDropwdown we are using fullTextSearch={{true}}
and what I found is this
Screenshot from 2020-11-16 04-04-00
so either the best option could be instead of using false use true and not changes the list of countries

@iamareebjamal
Copy link
Member

iamareebjamal commented Nov 16, 2020

That's not the issue. Please read it again, carefully. See the linked PR and the PR I have mentioned there

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 16, 2020

That's not the issue. Please read it again, carefully. See the linked PR and the PR I have mentioned there

@iamareebjamal I understood the issue, pls clear me out do I have to turn autocomplete='off' for entire form or just for country dowpdown bcz as you mentioned in tax-info-modal autocomplete='off' is for entire form.
And also should I revert back the changes made till now.
Please let me know this.

@iamareebjamal
Copy link
Member

The issue mentions in the title - only country

And I have mentioned it in the linked PR as well

@divyamtayal
Copy link
Member Author

@iamareebjamal Please see the changes made.

@iamareebjamal
Copy link
Member

Still getting autocomplete

@divyamtayal
Copy link
Member Author

Still getting autocomplete

@iamareebjamal can u pls share the screenshot

@divyamtayal divyamtayal changed the title Only show country dropdown, do not automatically fill in random info - only country fix: Only show country dropdown, do not automatically fill in random info - only country Nov 19, 2020
@auto-label auto-label bot added the fix label Nov 19, 2020
@iamareebjamal iamareebjamal changed the title fix: Only show country dropdown, do not automatically fill in random info - only country fix: Only show country dropdown without autocomplete Nov 20, 2020
@iamareebjamal iamareebjamal merged commit 70c2285 into fossasia:development Nov 20, 2020
@divyamtayal divyamtayal deleted the only-countries-5438 branch November 23, 2020 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants