twitter-clone
twitter-clone copied to clipboard
[WIP] Added location for Tweet input
Describe your changes
Hi there! Firstly thanks for open-sourcing this wonderful project. The whole app is cool and tidy.
So when exploring the app, I noticed that some of the Tweet input options were not implemented, so I was trying to implement the location feature.
The deployment is available at here.
Issue ticket number and link
#49
Checklist
- [Done]
locationinput option interaction - [Done]
locationedit modal layout & interaction(adding/editing/removing) - [Done]
locationdisplay inTweetinput form - [TODO] Inject world cities(format {city: STRING, country: STRING}) into Firestore (current location options are hard coded)
- [Done] Add
locationdata field toTweet - [Done] Integrate
locationintoTweetwhen creating/displayingTweetin view - [Done] Add mobile end display support
Someone is attempting to deploy a commit to a Personal Account owned by @ccrsxx on Vercel.
@ccrsxx first needs to authorize it.
I'm sure all contributions are welcomed :). Mention me once you have everything implemented. You also haven't implemented it for mobile yet either from what I can see.
The front-end component and interactions are almost done. Feel free to test it here.
However, currently for location select I'm still using a hard-coded {city: "city", country: "country"} list for users to select from. I need some suggestions on how we should store the data. There are few options:
- Create cities Document on Cloud Firestore (import from local static cities data CSV/JSON), but this needs to be done by the maintainer as I don't have access to the DB. Afterwards, I can implement the service and APIs to fetch the cities data in the component.
- Use geo location web service like Geonames, to query the cities data every time. I don't like this one as most free geo APIs have query limits.
What are your thoughts? @ccrsxx @Ketchupchh
You can probably just use Firebase for this. But there's probably a built in way to do this though I'm not sure lol :p
Also make sure you manage the overflow so we don't get things like this especially on mobile.
You can probably just use Firebase for this. But there's probably a built in way to do this though I'm not sure lol :p
Also make sure you manage the overflow so we don't get things like this especially on mobile.
Handling overflow on my end and have a rough fix now. But looks like TweetDate rendered twice on mobile end is a problem on the Master deployment here... I'll try to look for a fix for this first and include my overflow fix, or double dates won't leave any space for location....
Yes, I'm aware of this. I don't think it's necessarily a problem with the code but more so that the data changes depending on what device you're using but I could totally be wrong. Regardless, you should always wrap your overflow in case more things need to be added to that space in the future.
Yes, I'm aware of this. I don't think it think it's necessarily a problem with the code but more so that the data changes depending on what device you're using but I could totally be wrong. Regardless, you should always wrap your overflow in case more things need to be added to that space in the future.
Sounds good! I'll see if I can find a quick fix, otherwise I'll just wrap any overflow in place.
Fixed the Tweet Date double rendering issue and added my overflow fix on mobile end. Please check them if have time. :)
Everything looks fine on my end :). Last thing is to support all locations. Have you checked out the Firebase Geo queries yet?
Everything looks fine on my end :). Last thing is to support all locations. Have you checked out the Firebase Geo queries yet?
Yeah I'm looking into it. BTW, thanks for reviewing! I'll ping you once everything's done.
Hi thanks for the PR, currently I'm pretty busy with work and uni. I can take a look at the PR next week on the weekend.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| twitter-clone | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | May 9, 2024 10:24am |
Sorry, I'm just now looking at the PR, been pretty busy for the past three weeks.
So currently, the data for the location is stored locally right? For the location I think you can use Place API from Google Maps. You can get the location by address or city with this.
Or maybe just store the location locally if it just contains a city or a county, it should be pretty small.
I went to check the location feature on the real Twitter or X (what it's called now) to see what it looks like, but the option is greyed out, can't click the location. Hmm why is it disabled or are there some settings I need to enable to use this lol?
Hi there, sorry for the late response, I was busy with my intern the past 2 months.
For the location feature on the real Twitter, you can enable it in https://twitter.com/settings/location. It's actually weird that it allows you to add any location tags to your posts irrelevant with your real location. So I have also implemented the location tag in a way that user can select any location tags.
And yes, since the location tag feature only needs city and country info, I have stored the locations locally in a JSON file which is not too large.
Does this sound OK to you? @ccrsxx
No problem, I was busy too, and still am right now.
I have an uni exam in the coming weeks and a bunch of final projects next week after that. So I can review it maybe after the next two or three weeks later.
I took a quick look at your code, it looks ok. I'll review it in full by the time I'm free later.
@Altair59 Searching locations in the Location Modal hangs and crashes on mobile (at least for me. Also haven't checked on pc).
Not sure if this is related to this PR but when clicking and viewing a tweet, you get an application error now.
