twitter-clone icon indicating copy to clipboard operation
twitter-clone copied to clipboard

[WIP] Added location for Tweet input

Open Altair59 opened this issue 1 year ago • 16 comments

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] location input option interaction
  • [Done] location edit modal layout & interaction(adding/editing/removing)
  • [Done] location display in Tweet input form
  • [TODO] Inject world cities(format {city: STRING, country: STRING}) into Firestore (current location options are hard coded)
  • [Done] Add location data field to Tweet
  • [Done] Integrate location into Tweet when creating/displaying Tweet in view
  • [Done] Add mobile end display support

Altair59 avatar Mar 07 '24 09:03 Altair59

Someone is attempting to deploy a commit to a Personal Account owned by @ccrsxx on Vercel.

@ccrsxx first needs to authorize it.

vercel[bot] avatar Mar 07 '24 09:03 vercel[bot]

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.

Ketchupchh avatar Mar 07 '24 11:03 Ketchupchh

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:

  1. 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.
  2. 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

Altair59 avatar Mar 08 '24 05:03 Altair59

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. image

Ketchupchh avatar Mar 08 '24 10:03 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. image

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....

Altair59 avatar Mar 08 '24 12:03 Altair59

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.

Ketchupchh avatar Mar 08 '24 12:03 Ketchupchh

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.

Altair59 avatar Mar 08 '24 12:03 Altair59

Fixed the Tweet Date double rendering issue and added my overflow fix on mobile end. Please check them if have time. :)

Altair59 avatar Mar 09 '24 06:03 Altair59

Everything looks fine on my end :). Last thing is to support all locations. Have you checked out the Firebase Geo queries yet?

Ketchupchh avatar Mar 09 '24 21:03 Ketchupchh

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.

Altair59 avatar Mar 11 '24 01:03 Altair59

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.

ccrsxx avatar Mar 12 '24 14:03 ccrsxx

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

vercel[bot] avatar Mar 31 '24 16:03 vercel[bot]

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?

ccrsxx avatar Mar 31 '24 16:03 ccrsxx

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

Altair59 avatar Apr 29 '24 04:04 Altair59

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.

ccrsxx avatar May 09 '24 10:05 ccrsxx

@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.

image

Ketchupchh avatar May 21 '24 11:05 Ketchupchh