Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

Substancia
Copy link
Contributor

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
Copy link

vercel bot commented Oct 4, 2022

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
Copy link
Contributor

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

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

issue.mp4

plz check whether it is an issue or not

@aditya-singh9
Copy link
Owner

It also has some merge conflicts, please resolve them too

@aditya-singh9
Copy link
Owner

Please look into the preview

Screenshot_20221005-165719_Chrome.jpg

@aditya-singh9
Copy link
Owner

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

@Substancia
Copy link
Contributor Author

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

@Substancia Substancia mentioned this pull request Oct 5, 2022
@Rudresh-pandey
Copy link
Contributor

Rudresh-pandey commented Oct 5, 2022

Sorry bymistake i added the website footer code in mobile view

@Substancia
Copy link
Contributor Author

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
Copy link
Contributor

in my localhost :
image

@Substancia
Copy link
Contributor Author

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
Copy link
Contributor

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

@Substancia
Copy link
Contributor Author

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

@Rudresh-pandey
Copy link
Contributor

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
Copy link
Owner

I guess we should address these somewhere else.

@aditya-singh9 aditya-singh9 merged commit 2c1ee81 into aditya-singh9:main Oct 5, 2022
@Substancia Substancia deleted the bugfix/emoji-delete-bug-issue-61 branch October 5, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants