aditya-singh9 / kekfinder

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

bugfix/emoji-delete-bug-issue-61 #70

Closed Substancia closed 1 year ago

Substancia commented 1 year ago

Converted string of emojis into array of emojis.

This is a functionality fix, unaffected by upcoming UI changes. Can delay PR merge till after the UI change, will not cause merge conflicts either.

vercel[bot] commented 1 year ago

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

Name Status Preview Updated
kekfinder ✅ Ready (Inspect) Visit Preview Oct 5, 2022 at 2:33PM (UTC)
Rudresh-pandey commented 1 year ago

hey @Substancia i have seen the vecel development perview of this PR

sometimes while deleting the emoji's it deletes 2 instead of 1

https://user-images.githubusercontent.com/96531798/194036613-45b89f7b-8131-4938-9073-25157073157a.mp4

plz check whether it is an issue or not

aditya-singh9 commented 1 year ago

It also has some merge conflicts, please resolve them too

aditya-singh9 commented 1 year ago

Please look into the preview

Screenshot_20221005-165719_Chrome.jpg

aditya-singh9 commented 1 year ago

@Substancia when you are done, please leave a comment. Thanks

Substancia commented 1 year ago

@Rudresh-pandey can you please confirm on the latest commit whether this problem persists? I can't reproduce this.

@aditya-singh9 The footer seems to be not appropriately coded, and has an individual library used just for the footer into which I'll have to look into first. I think we can take it up under the footer issue or an entirely new issue maybe?

New commits have been made merging the UI changes, resolving merge conflicts, and reapplying changes. Please take a look.

Rudresh-pandey commented 1 year ago

Sorry bymistake i added the website footer code in mobile view

Substancia commented 1 year ago

Can we shift the footer conversation to the footer issue? That way we can decouple the problems and close (or discuss) only this bug here.

Rudresh-pandey commented 1 year ago

in my localhost : image

Substancia commented 1 year ago

My bad on the footer issue, somehow a position attribute got added accidentally while rebasing my branch. I've put it back to how it was.

Screenshot 2022-10-05 at 18 51 52 Screenshot 2022-10-05 at 18 52 04 Screenshot 2022-10-05 at 18 52 10

Looks like originally the footer won't stick to the bottom when the emojis list is empty. Do we want it that way?

Rudresh-pandey commented 1 year ago

@Substancia , @aditya-singh9 , @ArunBohra12 plz try to run this #72 in your local server and check whether it's working or not

Substancia commented 1 year ago

@Rudresh-pandey the desktop/mobile view switching is working on mine, but the footer isn't sticking when width < 1400px.

Rudresh-pandey commented 1 year ago

My bad on the footer issue, somehow a position attribute got added accidentally while rebasing my branch. I've put it back to how it was.

Screenshot 2022-10-05 at 18 51 52 Screenshot 2022-10-05 at 18 52 04 Screenshot 2022-10-05 at 18 52 10

Looks like originally the footer won't stick to the bottom when the emojis list is empty. Do we want it that way?

yes we want the footer at the bottom when list is empty in mobile view and tablet view

aditya-singh9 commented 1 year ago

I guess we should address these somewhere else.