alpaylan / afetbilgi.com

https://afetbilgi-com.vercel.app
70 stars 20 forks source link

Add city selection #193

Closed Kaanayden closed 1 year ago

Kaanayden commented 1 year ago

Resolves #142 Şehir seçimi için PDF indirme componentinde kullanılan Autocomplete componentinin temelini kullanıp sadece ana sayfadan seçilebilen bir şehir seçim aracı yazdım. Farklı dilleri de destekliyor. Eğer tıklanacak data türü 'question'sa, seçili şehrin URL'sini kullanıp kullanıcı direkt veriye yönlendiriyor. Eğer bütün şehirler için aynı veri varsa ortak sayfaya yönlendiriyor. Ayrıca seçilen şehir LocalStorage'a kaydediliyor ve sayfa kapandıktan sonra son şehir seçimi geri yükleniyor.

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
afetbilgi-com ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 4:24PM (UTC)
maps-afetbilgi-com ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 4:24PM (UTC)
vercel[bot] commented 1 year ago

@Kaanayden is attempting to deploy a commit to the alpaylan's Team Team on Vercel.

A member of the Team first needs to authorize it.

Kaanayden commented 1 year ago

Ek olarak ana sayfa dışında da şehir seçimi imkanı eklenebilir.

Ayb3rk commented 1 year ago

Elinize sağlık hocam. Bir kaç feedbackim olucak bu PR ile alakalı:

yigitv4rli commented 1 year ago

++ Mesela diyelim ben sehir olarak "Ankara" sectim. Varsayalim ki Ankara'da genel ihtiyaclar kisminda gecici barinma ve guvenli toplanma alani verisi yok. O case'de bu tableri display etmesek hos olabilir. Bir de ili secince Lütfen bilgi almak istediğiniz konuyu seçiniz. yerine {{ city }} ili hakkinda hangi konuda bilgi almak istiyorsunuz? gibi bir yaziya cevirebiliriz. Cunku ben sehir secince ne degistigini anlayamadim ilk seferde

Kaanayden commented 1 year ago

Hocam feedback için teşekkürler. Benim de birkaç sorum olacak? 1- https://cdn.afetbilgi.com/md-pdf/index.json linkindeki veriler için tekrar bir json dosyası oluşturayım mı? Çünkü sürekli fetch atmak bandwith kullanımı açısından iyi olmayabilir. 2- Şehir seçtikten sonra dediğiniz gibi yalnızca o şehirde var olan dataları göstermek mantıklı. Peki, bütün şehirler için aynı olan data'yı da göstermeli miyiz şehir seçtikten sonra? Diğer dediklerinizi de koda aktarmaya çalışacağım en kötü bu gece ya da yarın bitiririm diye düşünüyorum.

Ayb3rk commented 1 year ago

Hocam feedback için teşekkürler. Benim de birkaç sorum olacak? 1- https://cdn.afetbilgi.com/md-pdf/index.json linkindeki veriler için tekrar bir json dosyası oluşturayım mı? Çünkü sürekli fetch atmak bandwith kullanımı açısından iyi olmayabilir. 2- Şehir seçtikten sonra dediğiniz gibi yalnızca o şehirde var olan dataları göstermek mantıklı. Peki, bütün şehirler için aynı olan data'yı da göstermeli miyiz şehir seçtikten sonra? Diğer dediklerinizi de koda aktarmaya çalışacağım en kötü bu gece ya da yarın bitiririm diye düşünüyorum.

1- index.json dosyası halihazırda pdf için çekiliyor bu yüzden 2. kullanımda cache'den gelecektir. Ayrıca index.json dosyası yeni eklenen şehirlere göre otomatik olarak güncelleniyor. Bu yüzden s3 üzerinden çekmek daha mantıklı olur diye düşünüyorum. 2- Bu konuda emin olamadım ama benim şahsi fikrim göstermeye devam etmeliyiz yönünde. (@yigitv4rli buna sen de bir fikir belirtirsen iyi olur.)

yigitv4rli commented 1 year ago

Aynilarini yaziyordum, 1 icin bence de cache'ten dolayi sorun olmayacaktir. 2. icin bence de gosterelim.

Kaanayden commented 1 year ago

Hocam bahsettiğiniz feedbacklere göre eklemeler yaptım. Belirtmek istediğim iki şey olacak: 1- Şehirde hangi verinin olup olmadığına bakmak için Arrays.some fonksiyonu kullanıyorum. Eğer çok işlem yapacağını düşünürseniz site ilk yüklendiğinde "hangi veri hangi şehirde var?"ı tutmak için bir array ya da JSON objesi oluşturabiliriz. 2- Şehir seçimini kaldırmak için AutoComplete componentinde bir çarpı butonu var fakat mobil telefon gibi farklı kullanımlarda insanlar şehir seçiminin nasıl kaldırılacağını fark edemeyebilir. Belki şehir seçimini kaldırmak için ekstra bir buton daha ekleyebiliriz bu component'in yanına.

Kaanayden commented 1 year ago

Hocam bir de belirtmeyi unutmuşum main'e eklemeden önce locales klasöründe "page.main.subtitle_city" için çeviriler yapılması gerekiyor.

ozansz commented 1 year ago

Elinize saglik hocam. Fonksiyonalite olarak bence bir eksik yok. Fakat su an sitede 3 tane alt alta row'u olan bir header var ve bu yuzden asil icerik (basliklar) cok asagida kaliyor. Bu PR icinde bunlari da tek row'a alabilir miyiz? Header'i olabildigince dar yapsak guzel olur.

Bunun yerine:

Screen Shot 2023-02-15 at 16 45 20

Soyle bir sey olabilir:

Screen Shot 2023-02-15 at 16 47 48

cc: @Ayb3rk @yigitv4rli

Kaanayden commented 1 year ago

@ozansz Hocam dediğiniz gibi yaptım. Şu şekilde oldu: Ana Sayfa Veri Sayfası

yigitv4rli commented 1 year ago

1- Kaan cok sag ol, seni ugrastiricam tekrardan, su duzen daha mi hos? Senin dusuncen ne bir user olarak? Araya cok az bir padding ekleyebiliriz. Ben mobile mode gecip ss aldim o yuzden padding az oldu sanirim. [1] 2- Il secilince bir auto refresh atip url'e onu ekleyen bir sey ekleyebilir miyiz? Aklimdaki steps:

[1]

image

[2]

image
Kaanayden commented 1 year ago
  1. Ekran görüntüsündeki düzen mobilde diğer türlü gözükmediği için yapmıştım aslında. Ama bence yine de hem mobil hem masaüstünde şehir altta olunca daha iyi gözüküyor gibi geldi bana.
  2. Yapmaya başlayayım. Peki yeni sayfaya geçince tamamen refresh atıp goBack stack'ini filan da devre dışı bırakayım değil mi? Yoksa her şehir seçimi go back butonunu bozacak.
yigitv4rli commented 1 year ago
  1. Guzel nokta. Orayi sifirlayabiliriz bence yeni bir sehir secilirse. Ayni sekilde carpiya basip clear edince sehir secimini, yine refresh atip direkt afetbilgi.com'a yonlendiririz
Kaanayden commented 1 year ago

Padding'i biraz arttırıp city selection'u aşağı aldım.

Bilgisayar: Bilgisayar

Mobil: Mobill

yigitv4rli commented 1 year ago

Boyle daha iyi oldu sanki, begendim ben de

Kaanayden commented 1 year ago

Birkaç sorum daha olacak. 1- Eğer url sistemi kullanırsak son bakılan şehiri cache'de tutamayacağım, sorun olur mu acaba diye düşündüm de. 2- Bir de yeni şehire geçişte navigate'i mi kullanayım yoksa refresh mi atayım? Stack'te birikmeyi engellemek için.

yigitv4rli commented 1 year ago

1- Local Storageda tutabilirsin yine sorun olmayacaktir 2- Bunu benden daha iyi yorumlayacagi icin @Ayb3rk

Kaanayden commented 1 year ago

@yigitv4rli Fakat url sistemine geçtiğimiz için normal sayfaya girildiğinde herhangi bir şehir seçimi olmaması gerektiği için localstorage kullanmak mantıksız oluyor gibi.

Kaanayden commented 1 year ago

Hocam url'ye göre şehir seçimini yaptım, Ekstra bir feedback'iniz olursa yarın iletebilirsiniz.

Ayb3rk commented 1 year ago

Kaan ellerine sağlık gayet iyi görünüyor şuanki hali. Stackte birikmesi bir sorun yaratmıyor navigate edebiliriz, tek tek tüm şehirleri seçip baktım bir kasma yaşamadım sitede. Tek bir sorum olucak, şehir seçimi hala zorunlu gibi gözüküyor. Bunun önüne nasıl geçebiliriz acaba? @ozansz @yigitv4rli

oznakn commented 1 year ago

nasıl yani @Ayb3rk?

Kaanayden commented 1 year ago

@Ayb3rk Hocam ne demek istediğinizi tam anlayamadım da.

Ayb3rk commented 1 year ago

nasıl yani @Ayb3rk?

Siteye ilk girildiğinde karşıya direkt şehir seçimi çıkıyor ve ne işe yaradığı anlaşılmıyor. Seçmek zorunlu duruyor gibi göründü gözüme. Eğer herkese öyle gelmediyse düzeltmeye gerek yok

Kaanayden commented 1 year ago

nasıl yani @Ayb3rk?

Siteye ilk girildiğinde karşıya direkt şehir seçimi çıkıyor ve ne işe yaradığı anlaşılmıyor. Seçmek zorunlu duruyor gibi göründü gözüme. Eğer herkese öyle gelmediyse düzeltmeye gerek yok

Hocam bunun için 2 çözüm önerim var: 1- Şehir seçimi için olan component'te "Şehir seçiniz" yerine "Şehir", "Şehirler", ya da "Tüm Şehirler" yazabilir. Bu şekilde zorunlu olmadığı anlaşılabilir belki. 2- Title'daki "Lütfen bilgi almak istediğiniz konuyu seçiniz.", "Tüm şehirler hakkında" diye başlayabilir belki. Fakat bu çözümün iyi olup olmayacağından tam emin olamadım.

alpaylan commented 1 year ago

Hocam öncelikle ellerine sağlık. Şunun doğru UX'ine karar vermeye çalışıyorum bir başarabilirsek seni de yormadan ya mergeleyelim ya da ona göre değiştirelim istiyorum. Bir süre uyuyor kalabilir burası o yüzden özür dilerim.

ozansz commented 1 year ago

bunu artik mergeleyelim diyorum UX update'i sonra da yapariz cc: @Ayb3rk @yigitv4rli @oznakn @alpaylan

oznakn commented 1 year ago

ben de aynı fikirdeyim

alpaylan commented 1 year ago

Eğer sectionın altı boşsa alignment kayıyor. Onu düzelttikten sonra approve layayım hak verdim ben de ux update yakında gelmeyecek gibi

image
oznakn commented 1 year ago

@Kaanayden hocam siz ilgilenebilir misiniz yoksa biz bakalım mı?

Kaanayden commented 1 year ago

Eğer sectionın altı boşsa alignment kayıyor. Onu düzelttikten sonra approve layayım hak verdim ben de ux update yakında gelmeyecek gibi

image

Hocam o "diğer" sekmesinde oluyordu zaten sadece. O sekmeyi kaldırmışsınız sanırım yeni main'e mergeledikten sonra öyle bir sorun kalmadı.