ekwonye-richard / react-flags-select

Customizable svg flags select components for React Js
194 stars 121 forks source link

Allows showing multiple times the same country (Closes #61) #63

Closed zedtux closed 3 years ago

zedtux commented 3 years ago

This PR allows to show multiple times the same country flag but with a different label (a use case is French in Belgium and Dutch in Belgium) and it also removes the customLabels property, which was doing some redundancy with countries, and reworks the way countries is managed.

This PR also updates the README.md file in order to show case showing multiple times the same country. It also updates the demo source file.

customLabels deletion

BREAKING CHANGE

It was much more simple to transform the countries array of string in an array of object in order to pass the custom labels for each countries, but this PR supports both formats so that you can still do:

<ReactFlagsSelect
  countries={["US", "GB", "FR","DE","IT"]}
/>

And now you can also do:

<ReactFlagsSelect
  countries={[{"US": "EN-US"}, {"GB": "EN-GB"}, {"FR": "FR"}, {"BE": "FR"}, {"BE": "NL"}, {"IT": "IT"}]}
/>

I was thinking of implementing a deprecation warning mechanism in this library and deprecate the customLabels property, but I ended up not doing it for the following reason: Who reads browser's warnings? Instead, people using customLabels will got an error (so they'll look at it), and coming to this repo's README.md file, they will understand how to migrate. A short description on how to migrate could be added to the to of the README.md file if you think this is too hard.

Multiple times the same country

Basically before this PR, the state selected attribute was storing the country code, BE in my case. Now it stores the position in the countries array.

The onSelect and onSelectWithKeyboard functions have been updated in order to support both countries property format and now works like that:

Finally the searchable function has been adapted too in order to continue searching the countries list from the countries.js file when the countries property is an array of strings, but searches the custom label from the countries property when the countries property is an array of objects.

The things I've tested

blackList

This excludes the given entries to the countries properties and show a long list of flags.

defaultCountry

The defaultCountry when not found is just not selecting an entry (I tried to set it to GB while it is not in the countries list), but when passing an existing value, it pre-selected it.

showSelectedLabel

No big deal, show/hide the label as expected.

showOptionLabel

No big deal, show/hide the label as expected.

onSelect

I tried to use twice this library in the same page:

Both was binding the onSelect to a dedicated function showing a different console.log.

Im the first case I received BE or FR in my case, and in the second case I received BE_FR, BR_NL, and FR_FR.

searchable

Searching nl showed only BE_NL entry, while searching fr showed BE_FR and FR_FR entries.

ekwonye-richard commented 3 years ago

Hi @zedtux Sorry I'm coming to this super late. I've been off the repo for a while. I like this approach for handling labels and allowing multiple display of same flag. I'm currently working on V2 which will handle a lot of issues the library currently has. I'll be doing some beta releases in the coming days, first with the exact functionality of v1, but with updated version of react, supporting types and now making this a controlled component. I will love to implement this logic before releasing a stable version of v2 especially since it's a break in change. I'm also trying to make contributing much more easier. Let me know if you will like to work in this implementation on v2 or I can pick it up myself.

zedtux commented 3 years ago

Hi @ekwonye-richard,

Thank you for coming back to me about this one :).

I made this PR in a company's project which is now over, and now I'm working on another so if you could take it, I would prefer, but I can help on testing the new version 😉.

Happy new year.

ekwonye-richard commented 3 years ago

No worries @zedtux I'll pick this up. I'll also let you know when it's pushed so you can test be before I close this PR.

ekwonye-richard commented 3 years ago

@zedtux on a second thought and taking closer look, this approach may not be best on v2. I am making this a controlled input where country code value is passed down from the parent component and the value can be updated with the onSelect prop. The select value is highly dependent on the countryCode, hence it has to remain unique.

We can come up another approach to reuse flags, but that will come in as an update to v2 if we get more requests for it.