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

Filter Timezones by cities #71

Open
Toledodev opened this issue Jun 14, 2022 · 48 comments
Open

Filter Timezones by cities #71

Toledodev opened this issue Jun 14, 2022 · 48 comments

Comments

@Toledodev
Copy link

Toledodev commented Jun 14, 2022

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
Copy link
Owner

ndom91 commented Jun 15, 2022

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
Copy link

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
Copy link
Owner

ndom91 commented Jun 15, 2022

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
Copy link
Owner

ndom91 commented Jun 17, 2022

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
Copy link

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
Copy link
Author

I can try, since I was already trying to do

@ndom91
Copy link
Owner

ndom91 commented Jun 20, 2022

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
Copy link

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
Copy link

PeerRich commented Oct 3, 2022

I can try, since I was already trying to do

@Toledodev any update on this?

@Toledodev
Copy link
Author

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

@Toledodev
Copy link
Author

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
Copy link
Author

What do you think? @PeerRich

@PeerRich
Copy link

PeerRich commented Oct 6, 2022

where is the link?

@Toledodev
Copy link
Author

Toledodev commented Oct 26, 2022

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
Copy link
Owner

ndom91 commented Oct 27, 2022

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 😁

@Toledodev
Copy link
Author

Toledodev commented Oct 27, 2022

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
Copy link

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
Copy link
Owner

ndom91 commented Nov 1, 2022

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:

  • Debounce input
  • After 2-3 chars start searching the list of available cities
  • Recommend the top 3-5 results
  • When one is selected, match it with the timezone and return the same (or similar) timezone object as before.

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

@PeerRich
Copy link

PeerRich commented Nov 1, 2022

@ndom91 that sounds awesome.

@Toledodev wanna build this? we can sponsor this feature

@Toledodev
Copy link
Author

Of course, you can count on me 👍

@Toledodev
Copy link
Author

Toledodev commented Nov 1, 2022

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

@ndom91
Copy link
Owner

ndom91 commented Nov 1, 2022

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
Copy link
Author

Toledodev commented Nov 3, 2022

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
Copy link
Owner

ndom91 commented Nov 3, 2022

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
Copy link
Author

I agree with you, thanks for the help 👍

@Toledodev
Copy link
Author

Toledodev commented Nov 4, 2022

@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
Copy link

PeerRich commented Nov 4, 2022

@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
Copy link
Owner

ndom91 commented Nov 4, 2022

@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
Copy link

PeerRich commented Nov 6, 2022

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

@Toledodev
Copy link
Author

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
Copy link

PeerRich commented Nov 8, 2022

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

@ndom91
Copy link
Owner

ndom91 commented Nov 9, 2022

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
Copy link
Author

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
Copy link
Owner

ndom91 commented Nov 10, 2022

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
Copy link

PeerRich commented Feb 8, 2023

is anyone actively working on this?

would love to pay for it, too

CleanShot.2023-02-08.at.15.09.21.mp4

@ndom91
Copy link
Owner

ndom91 commented Feb 10, 2023

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
Copy link
Author

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
Copy link

PeerRich commented Feb 10, 2023

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
Copy link
Author

@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
Copy link

ah yes that looks correct!!

@Toledodev
Copy link
Author

Ok, that's nice 😄, I will add more tests and send the PR

@ndom91
Copy link
Owner

ndom91 commented Feb 10, 2023

Woo! Awesome thanks a ton @Toledodev

@PeerRich
Copy link

@ndom91 we actually sponsored this PR ❤️

@PeerRich
Copy link

@Toledodev do you raise the PR here right?

@ndom91
Copy link
Owner

ndom91 commented Dec 10, 2023

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

@djshubs
Copy link

djshubs commented Jan 22, 2024

Any word on this?

@PeerRich thanks for sponsoring this functionality.

@nik32
Copy link

nik32 commented Mar 7, 2024

@ndom91 is this feature available in the latest versions?

@ndom91
Copy link
Owner

ndom91 commented Mar 8, 2024

@nik32 no it's not available yet

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

No branches or pull requests

5 participants