ndom91 / react-timezone-select

🌐 An extremely usable and dynamic React timezone selector
https://ndom91.github.io/react-timezone-select/
MIT License
208 stars 46 forks source link

Filter Timezones by cities #71

Open Toledodev opened 2 years ago

Toledodev commented 2 years ago

What i tried to do? .I was trying to filter the timezones by cities, type the name of a city and be rendered in the dropdown the city or the corrersponding timezone, it is possible? I noticed that the library doens't have data besides the timezones. .I even tried to create a relationship between this library and another that provides the data of the citeis but it was not possible, becausa the "Timezones" prop does not accept anything different from what it already has in it

Example: .If i type "San Diego" in the input it should suggest the San Diego timezone, the suggestion can be either to render at once the timezone in which the city exists. This is how it is now: Screenshot from 2022-06-14 10-21-38 https://user-images.githubusercontent.com/87545086/173698851-637d3fb1-1ce5-4205-bcda-736f09bb1626.mp4

Dont' have any suggestions about the city(the value on input)

Could have a feature to make it possible to put more name options for the same timezone. So it would be possible to filter by city too(if the code wants to show these custom cities)

ndom91 commented 2 years ago

Yeah unfortunately it just filters by values that are in the drop-down, i.e. the predefined city combinations.

I don't want to add a dynamic "all city timezone lookup" to this drop-down filtering.

But you can add custom cities, see the "custom timezones" section in the readme πŸ‘

PeerRich commented 2 years ago

we use react-timezone-select at https://github.com/calcom/cal.com and would love to have a way to put cities in the search bar, but not show them in the dropdown (only once you start typing).

I don't want to add a dynamic "all city timezone lookup" to this drop-down filtering.

why not?

would you accept a PR if we build this for react-timezone-select?

ndom91 commented 2 years ago

we use react-timezone-select at https://github.com/calcom/cal.com and would love to have a way to put cities in the search bar, but not show them in the dropdown (only once you start typing).

I don't want to add a dynamic "all city timezone lookup" to this drop-down filtering.

why not?

would you accept a PR if we build this for react-timezone-select?

Yeah so I'm against accepting a PR πŸ˜… , my only concern is slowing down typing / filtering if it's constantly searching for cities/timezones

ndom91 commented 2 years ago

we use react-timezone-select at https://github.com/calcom/cal.com and would love to have a way to put cities in the search bar, but not show them in the dropdown (only once you start typing).

I don't want to add a dynamic "all city timezone lookup" to this drop-down filtering.

why not?

would you accept a PR if we build this for react-timezone-select?


Yeah so I'm against accepting a PR πŸ˜… , my only concern is slowing down typing / filtering if it's constantly searching for cities/timezones

@PeerRich Sorry, typo, NOT against πŸ˜‚

PeerRich commented 2 years ago

we use react-timezone-select at https://github.com/calcom/cal.com and would love to have a way to put cities in the search bar, but not show them in the dropdown (only once you start typing).

I don't want to add a dynamic "all city timezone lookup" to this drop-down filtering.

why not?

would you accept a PR if we build this for react-timezone-select?


Yeah so I'm against accepting a PR πŸ˜… , my only concern is slowing down typing / filtering if it's constantly searching for cities/timezones

@PeerRich Sorry, typo, NOT against πŸ˜‚

that edit makes a lot more sense 🀣

cool if we fix this for ourselves we will contribute it back to this repo via a PR πŸ‘

Toledodev commented 2 years ago

I can try, since I was already trying to do

ndom91 commented 2 years ago

Yeah, that'd be great! If you guys submit a PR i'd be happy to take a look and get it over the finish line together πŸ‘

PeerRich commented 2 years ago

I can try, since I was already trying to do

@Toledodev that would be wonderful!

I bet @ndom91 and me can split the bounty πŸ€‘

PeerRich commented 1 year ago

I can try, since I was already trying to do

@Toledodev any update on this?

Toledodev commented 1 year ago

Hey @PeerRich , I created a pr that gives when developing the possibility to add custom cities in the code

Toledodev commented 1 year ago

The react-timezone-select library has its timezones with their default values, by adding an "extraLabels" prop it is possible to link a city with its specific timezone and render it in the dropdown

Toledodev commented 1 year ago

What do you think? @PeerRich

PeerRich commented 1 year ago

where is the link?

Toledodev commented 1 year ago

Hey @PeerRich , sorry for the delay, when I opened the pr in the react-timezone-select repository I got this answer, do you think this already works for what you want

ndom91 commented 1 year ago

Ah sorry, didn't realize that was related to this, @Toledodev. You'd just mentioend adding custom labels in that other one. Lets not splinter the discussion across multiple issues.

Let us think about what the goal(s) really are here, because maybe I'm misunderstanding too.

We want to be able to recommend timezones based off of many more cities than are currently available in the dropdown (merged city/timezone JSON blob), right?

So for example, if someone types in a small town not in the list, like Pittsburgh or Stuttgart, we'd need to be able to predict which city they're entering based off the first few letters. So as they're typing - say they've typed Pit, then we'd have to lookup all cities that start with those three letters and their timezones. We can then provide those as autocomplete options..

That would be a "fully dynamic" solution. However, that comes with tons of network requests, debouncing logic, other headaches..

Alternatively, we could prepare a larger list of precompiled cities and their timezones (say all cities over 100k residents) and ship it with the library. That would make autocomplete easy, as we have all the possible options and their matching timezones ahead of time already. Of course we couldn't match on any possible city in that case, but I think thats a decent trade-off to make in order to avoid having to make tons of network requests to some city-lookup API, debounce those, etc. all just for a timezone dropdown component.

What do yall think?

I think the precompiled list of cities with mathcing timezones is a pretty decent solution tbh. Any other ideas? I'm all ears :grin:

Toledodev commented 1 year ago

Hey @ndom91, I think the second option would be simpler and would give less problems. Anyway I found this library that displays many cities and returns a "timezone" key for each city. Maybe we could relate this library to the existing timezones in react-timezone-select. The user would select a city after searching for it in the dropdown and we would return the correct timezone

PeerRich commented 1 year ago

Alternatively, we could prepare a larger list of precompiled cities and their timezones (say all cities over 100k residents) and ship it with the library. That would make autocomplete easy, as we have all the possible options and their matching timezones ahead of time already. Of course we couldn't match on any possible city in that case, but I think thats a decent trade-off to make in order to avoid having to make tons of network requests to some city-lookup API, debounce those, etc. all just for a timezone dropdown component.

definitely this, no network request needed

ndom91 commented 1 year ago

Hey @ndom91, I think the second option would be simpler and would give less problems. Anyway I found this library that displays many cities and returns a "timezone" key for each city. Maybe we could relate this library to the existing timezones in react-timezone-select. The user would select a city after searching for it in the dropdown and we would return the correct timezone

Okay great, yeah that seems to do the work of precompiling the list for us!

Could you give it a shot adding that functionality in a branch? Do you need any other changes from me?

I guess high-level, the happy path could look like this:

What do you think? That's just the first thing that came to mind for me, but I'm open to suggestions!

PeerRich commented 1 year ago

@ndom91 that sounds awesome.

@toledodev wanna build this? we can sponsor this feature

Toledodev commented 1 year ago

Of course, you can count on me πŸ‘

Toledodev commented 1 year ago

Hey @ndom91, how are u? In case I need how can I change the dropdown style?

ndom91 commented 1 year ago

Hey @ndom91, how are u? Jn case I need how can I change the dropdown style?

The dropdown is completely built on react-select so you'd have to override their styles.

Toledodev commented 1 year ago

Hey @ndom91, how are you? I'm trying to deploy this but I'm having a problem. I've already managed to use the city-timezones library and relate the timezones. The problem is that cities have different timezones. For example, "Juiz de Fora" is in Brazil and has the timezone "America/SaoPaulo", so it would work, but "Juanjui" has the timezone "America/Lima". Then the site breaks. Do you think we could add these other options to the timezone list? We would only use the ones that already exist now and when the user typed, the other timezones would be shown according to the filtering

Demo: https://www.loom.com/share/89fd8a8d123d452f9ea083bce5c8b252

As you can see, when trying to render in the dropdown other timezones that are not in the "allTimeZones" list, the site breaks. I put a console.log in the object with the timezones that should be shown

ndom91 commented 1 year ago

Hey @ndom91, how are you? I'm trying to deploy this but I'm having a problem. I've already managed to use the city-timezones library and relate the timezones. The problem is that cities have different timezones. For example, "Juiz de Fora" is in Brazil and has the timezone "America/SaoPaulo", so it would work, but "Juanjui" has the timezone "America/Lima". Then the site breaks. Do you think we could add these other options to the timezone list? We would only use the ones that already exist now and when the user typed, the other timezones would be shown according to the filtering

Demo: https://www.loom.com/share/89fd8a8d123d452f9ea083bce5c8b252

As you can see, when trying to render in the dropdown other timezones that are not in the "allTimeZones" list, the site breaks. I put a console.log in the object with the timezones that should be shown

Hey I'm well. Thanks for getting this done!

So in regard to matching the cities against my existing timezone json blob - we don't have to use my existing one. We can just look up America/Lima in the available timezones in spacetime/timezone-soft (to get the GMT offset, short code, etc.) and just return that.

What do you think?

Toledodev commented 1 year ago

I agree with you, thanks for the help πŸ‘

Toledodev commented 1 year ago

@ndom91 @PeerRich Please take a look at my video, what do you think? I think it's ready, I just need to solve the typescript problems https://www.loom.com/share/ba52667340144e60a9321cad720080d1

PeerRich commented 1 year ago

@ndom91 @PeerRich Please take a look at my video, what do you think? I think it's ready, I just need to solve the typescript problems https://www.loom.com/share/ba52667340144e60a9321cad720080d1

this looks great!

ndom91 commented 1 year ago

@ndom91 @PeerRich Please take a look at my video, what do you think? I think it's ready, I just need to solve the typescript problems https://www.loom.com/share/ba52667340144e60a9321cad720080d1

Yeah this is great!

My only feedback would be, when typing in "San Francisco" and selecting that recommendation, instead of displaying "America/Los Angeles" then, maybe display "(GMT+7:00) San Francisco" or something like that.

PeerRich commented 1 year ago

Yeah i think it makes sense to then use the City and not change to the timezone name

Toledodev commented 1 year ago

But then the format would be different from the accepted one, I'm not sure if we could do this, and if we keep it as it is in the loom video we would follow the pattern, what do you think? Screenshot_9

PeerRich commented 1 year ago

ok in that ase we can keep it this way for now and maybe follow up

ndom91 commented 1 year ago

Yeah just wanted to chime in, I'm totally open to changing the format of the label/options. As long as it's user friendly and not a machine label like "Europe/Berlin".

This change is significant enough, that I'm totally open to making it a major version bump and cutting and breaking πŸ‘

Toledodev commented 1 year ago

Hey @ndom91, I'm working on another pr related to timezoneSelect and I think that when we put the two together we will have the expected result Screenshot_12

ndom91 commented 1 year ago

Hey @ndom91, I'm working on another pr related to timezoneSelect and I think that when we put the two together we will have the expected result Screenshot_12

Okay great, just keep me posted. Thanks a lot for this btw!

PeerRich commented 1 year ago

is anyone actively working on this?

would love to pay for it, too

https://user-images.githubusercontent.com/8019099/217553278-721fcf9f-942d-4427-828d-4f9a468e0e95.mp4

ndom91 commented 1 year ago

I am not working on it actively, I can say that much. @Toledodev were you able to make any progress? Is the work in the loom you shared previously available somewhere? Or you still have it locally only?

Toledodev commented 1 year ago

Hey @ndom91 @PeerRich I opened a PR yesterday, I'm just adding some tests Please take a look: https://www.loom.com/share/c80d504aea174a9496859d9f43819912

PeerRich commented 1 year ago

hey, this is not correct yet.

the default dropdown should have the official timezones only, but then when you search it should show up

edit: nice work

Toledodev commented 1 year ago

@PeerRich Can you please check if the behavior is right in this video? I checked in the main branch what the default timezones are and it seems to be correct, but I might be confusing πŸ˜„ https://www.loom.com/share/bd5034ef0d5a4c0fbfa982dd50b5a648

PeerRich commented 1 year ago

ah yes that looks correct!!

Toledodev commented 1 year ago

Ok, that's nice πŸ˜„, I will add more tests and send the PR

ndom91 commented 1 year ago

Woo! Awesome thanks a ton @Toledodev

PeerRich commented 1 year ago

@ndom91 we actually sponsored this PR ❀️

PeerRich commented 1 year ago

@Toledodev do you raise the PR here right?

ndom91 commented 7 months ago

@Toledodev I never saw a PR on this repo, did you raise it against something else?

djshubs commented 5 months ago

Any word on this?

@PeerRich thanks for sponsoring this functionality.

nik32 commented 4 months ago

@ndom91 is this feature available in the latest versions?

ndom91 commented 4 months ago

@nik32 no it's not available yet