aditya-singh9 / kekfinder

Emoji finding tool
https://kekfinder.vercel.app
MIT License
27 stars 17 forks source link

New UI and searched results #46

Closed Rudresh-pandey closed 1 year ago

Rudresh-pandey commented 1 year ago

Hi, the UI of KekFinder now looks odd to me , the searched items in a row are now only two and there is another feature of copying multiple emoji's which is great but now we have scroll more to find our desired emoji.

Current View :

After adding the above feature --- image

This has also some issue like overlaping of emoji's and multiple select features. which had already been noticed . . .

I would like to propose new UI to our project :

Light Theme :

image

Dark Theme :

image

And another suggestion on what to show when nothing is searched -- Rather than creating favourite section or showing all the emoji's , We can show all the previously copied emoji's to the user so that we don't have to bother about the number of emoji's to be displayed.

I am giving the link to the Figma design , feel free to make changes and present your idea https://www.figma.com/file/MLuUOQ38c7UC3Qh7dT0ekc/kekFinder?node-id=5%3A31 :)

aditya-singh9 commented 1 year ago

The part of, what to di when nothing is searched is being covered by another contributor so you dont have tk worry about that You can start working on the ui fixes

Rudresh-pandey commented 1 year ago

working on UI :)

Substancia commented 1 year ago

Hey @Rudresh-pandey is this a critical work, and if I proceed with https://github.com/aditya-singh9/kekfinder/issues/41, will it be breaking changes to your work?

Rudresh-pandey commented 1 year ago

Yes let me do the changes first .

Substancia commented 1 year ago

Sure. Let me know once you're done, and I'll proceed.

Rudresh-pandey commented 1 year ago

bhai , hum Ui improve kar diye h lekin ab ek do orr merge ho gaya ab kaise PR bheje

aditya-singh9 commented 1 year ago

Iguess you have to sync your fork

Or i think make a new branch and push the changes to that branch and then create a pr

I do no know exactly what we have to do.

aditya-singh9 commented 1 year ago

Did you fix it?

I wont merge anything from now till i merge your pr

Rudresh-pandey commented 1 year ago

bro , ye changes nhi ho paaega kuki jab starting mai user enter karga tab emoji's 0 rahega or tab UI thik nhi laega

image

isse leye close kar diye isko :) sorry message thoda late se kar rahe hai

aditya-singh9 commented 1 year ago

It is being taken care by @Substancia at https://github.com/aditya-singh9/kekfinder/issues/60 so don't worry about it make a pr

Rudresh-pandey commented 1 year ago

bro wo tho tab show hoga jab user pahle se emoji's ko pahle se copy kiya ho ### preview check kar https://kekfinder-r8w9xyvyu-aditya-singh9.vercel.app/

aditya-singh9 commented 1 year ago

Now see the website. Now it is showing

Now make a pr with ur cool design.

Rudresh-pandey commented 1 year ago

I will start working on this from tomorrow , So now you can merge others code's , But from tomorrow plz try not to merge otherwise it may create merge conflicts.

Probably this will take 1 day of work

Great work everyone :)

aditya-singh9 commented 1 year ago

Okay i wont merge anycode in this repo tmrw until your pr is made and merged

Substancia commented 1 year ago

There shouldn't be any merge conflicts if we are periodically syncing our fork with the upstream (or original repo). Technically blocking other PRs isn't ideal and goes against the concept of Git, but since we don't have many critical works running in parallel right now, I guess this is doable.

My point is, please periodically sync your fork with the original repo to avoid merge conflicts, so other devs won't have their work blocked. Cheerio!

aditya-singh9 commented 1 year ago

@Rudresh-pandey now please sync the repo and then start working on it

Rudresh-pandey commented 1 year ago

phone view wala code kidhr hai? desktop wala pe changes kiye tho mobile view wala change ho ja raha h uska code kaha hai?

aditya-singh9 commented 1 year ago

What do you mean by the phone code?

I do not know where it is because i havent seen the project for 10month

It must be in some css file

Rudresh-pandey commented 1 year ago

ok

Rudresh-pandey commented 1 year ago

@aditya-singh9 bro laptop ka screen mai thik dikh raha h ,

https://user-images.githubusercontent.com/96531798/193795935-c9d59775-34a9-4589-966d-e2b081985c5d.mp4

Lekin responsive nhi ho raha

https://user-images.githubusercontent.com/96531798/193796047-48bf89e3-9848-45a3-b91b-6ad9b489ebd9.mp4

Responsive hone ka code node modules mai hai usse kasie karte hai mujhe nhi pata i guess iss issue ko close karke dusra method dhundna hoga wo sticky wala issue ko solve karne ke leye #64 ya koi orr responsive code pe kaam kar sakta hai agar tho . @Substancia @ArunBohra12 any suggestions ?

Substancia commented 1 year ago

One way to tackle this is creating responsiveness using CSS media queries. You can define a screen width which differentiates different devices, and style the container accordingly for each screen.

As for mobile view, maybe we can stick with old design of having emojis below? In which case the sticky section issue will be needed to be addressed.

aditya-singh9 commented 1 year ago

One way to tackle this is creating responsiveness using CSS media queries. You can define a screen width which differentiates different devices, and style the container accordingly for each screen.

As for mobile view, maybe we can stick with old design of having emojis below? In which case the sticky section issue will be needed to be addressed.

Yes iguess in the mobile view, we should stick to the older design

ArunBohra12 commented 1 year ago

Yes IMO, media queries will be the best.

ArunBohra12 commented 1 year ago

Lekin responsive nhi ho raha Responsive hone ka code node modules mai hai usse kasie karte hai mujhe nhi pata i guess iss issue ko close karke dusra method dhundna hoga wo sticky wala issue ko solve karne ke leye #64 ya koi orr responsive code pe kaam kar sakta hai agar tho . @Substancia @ArunBohra12 any suggestions ?

Where do you see the code in node modules?

Rudresh-pandey commented 1 year ago

image

and ToastContainer using toastContainer.scss (node modules -> react-toastify -> scss -> animations )

Rudresh-pandey commented 1 year ago

yes mobile view will have the older design but when i changed the code for newUI (only for desktop view) it also changes the mobile view . Or maybe i should use % in styling rather than fixed parameters , and also implement media - queries

Substancia commented 1 year ago

image

and ToastContainer using toastContainer.scss (node modules -> react-toastify -> scss -> animations )

These are certain default styles coming from certain packages, and are overridden by the css files that we define, don't have to worry about them IF you're writing the styles that we need (just pretend that the default media queries don't exist and start writing).

TL;DR: write your own media queries for responsiveness and these styles from node modules will disappear.

Rudresh-pandey commented 1 year ago

ok thanks :)

Rudresh-pandey commented 1 year ago

Have a look :-

https://user-images.githubusercontent.com/96531798/193889062-9020a03c-d531-4388-9e46-b43258e78055.mp4

This will solve some issues :

1 . The text on the search bar is not visible completely

Old View: image

New View: image

2 . The issue of the sticky search bar and multiple select feature(msf) - in desktop view . We dont't have to worry about the search bar and msf in mobile view because we have a feature ScrollToTop (see the video)

3 . Number of emoji's in a row : more to see less to scroll

Old View : image

New View : image

thanks - @aditya-singh9 @Substancia @ArunBohra12 , for helping me out

Substancia commented 1 year ago

Good job @Rudresh-pandey.

"2 . The issue of the sticky search bar and multiple select feature(msf) - in desktop view . We dont't have to worry about the search bar and msf in mobile view because we have a feature ScrollToTop"

Tbh I'd prefer as a user to have the selected emojis section visible at all times, including when we scroll down, for the ease of keeping track what all emojis I've selected so far. This way, I can also keep track of misclicks and wrong emojis and immediately delete the wrong emojis from the selection. In which case, the sticky situation has to be taken care of as well.

Otherwise, the UI looks good. What does @aditya-singh9 think?

Rudresh-pandey commented 1 year ago

Yes we have to keep track of emoji's . Working on it :)

aditya-singh9 commented 1 year ago

I really appreciate his work and i really like the ui too.

And for the stickness, i am not sure, what should we do. Like should we keep it or remove it

Rudresh-pandey commented 1 year ago

Can i merge the current code then we should have a discussion on how to solve the stickyness issue :)