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

feat: added button to stop searching for a buddy #208

Merged
merged 3 commits into from
Oct 26, 2022
Merged

Conversation

ays14
Copy link
Contributor

@ays14 ays14 commented Oct 26, 2022

Fixes Issue

closes #132

πŸ‘¨β€πŸ’» Changes proposed(What did you do ?)

Created a button titled as Stop in BuddyMatcher.jsx which is a link to / and redirects you to home on click. Since, there is an effect with the callback that closes the socket and does necessary steps when component unmounts. Therefore, I need not worry about the same when redirecting to / with react-router.

βœ”οΈ Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • This PR does not contain plagiarized content.
  • The title and description of the PR is clear and explains the approach.

Note to reviewers

πŸ“· Screenshots

image

@vercel
Copy link

vercel bot commented Oct 26, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @Dun-sin on Vercel.

@Dun-sin first needs to authorize it.

@ays14
Copy link
Contributor Author

ays14 commented Oct 26, 2022

@Dun-sin there you go, for your convention a different branch. I hope you will update your rules to ask contributors to follow the convention for your repository.

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

@Dun-sin there you go, for your convention a different branch. I hope you will update your rules to ask contributors to follow the convention for your repository.

I did, don't know if you mean a rephrase because here it is πŸ‘‡πŸΎ
image

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

This PR doesn't fix the issue, the user is still on the waiting line for connecting with someone else, it has to totally stop, don't know if it's a miscommunication, but this PR should fix the following:

  • remove the user was the waiting queue array in the backend
  • send successful using socketio as used in this project to the frontend
  • then changes the route back to the root

@ays14
Copy link
Contributor Author

ays14 commented Oct 26, 2022

This PR doesn't fix the issue, the user is still on the waiting line for connecting with someone else, it has to totally stop, don't know if it's a miscommunication, but this PR should fix the following:

  • remove the user was the waiting queue array in the backend
  • send successful using socketio as used in this project to the frontend
  • then changes the route back to the root

Oh okay, let me figure this out to an approach and ping you before going ahead on code. Thanks for clearing it out.

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

@ays14 what you just did with the contributing file is none of this PRs business, so kindly delete and create a separate issue

@ays14
Copy link
Contributor Author

ays14 commented Oct 26, 2022

Approach:

  • send an API call to execute a function at backend a
  • a will then update the waiting list of users and emit a socket conn to message UI about successful removal
  • @Dun-sin what is the scale of your application? Depending on the scale, the removal has to be a in-place or new array.
  • UI will then redirect to the root after receiving a success.

@ays14
Copy link
Contributor Author

ays14 commented Oct 26, 2022

@ays14 what you just did with the contributing file is none of this PRs business, so kindly delete and create a separate issue

This was already removed within seconds after the push.

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

It has a backend, frontend, and a database
this line is where all the users searching are being stored

const waiting_users = {};

there's already a function to handle removing a user from the searching object

Whisper/server/lib.js

Lines 423 to 425 in 421ebac

function delWaitingUser(id) {
delete waiting_users[id];
}

delActiveUser,

you just need to create a socket event to detect when a user clicks the stop button
then handle the event in the backend, sending another event on successful to the frontend

@ays14
Copy link
Contributor Author

ays14 commented Oct 26, 2022

@Dun-sin does this look good?

server: index.js

socket.on('stop_search', async ({ loginId, email }) => {
        await delWaitingUser(loginId ?? email);
        socket.emit('stop_search_success');
    });

client: BuddyMatcher.jsx

    const handleStopSearch = () => {
        socket.emit('stop_search', { loginId: auth.loginId, email: auth.email });
        setIsStoppingSearch(true);
    };

    useEffect(() => {
        setLoadingText(isStoppingSearch ? stoppingSearchLoadingText : defaultLoadingText);
    }, [isStoppingSearch]);
...
   useEffect(() => {
...
        socket.on('stop_search_success', () => {
            navigate('/');
        });
...
, [])

return isFound ? ... :
{!isStoppingSearch && <button
                onClick={handleStopSearch}
                className={
                    'hover:no-underline hover:text-white font-medium text-white text-[1.5em] w-[8em] h-[2.3em] mt-4 rounded-[30px] border-4 border-solid border-[#f04336] flex flex-col items-center justify-center'
                }
            >
                Stop
            </button>}

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

looks good

@vercel
Copy link

vercel bot commented Oct 26, 2022

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated
whisper βœ… Ready (Inspect) Visit Preview Oct 26, 2022 at 0:20AM (UTC)

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

Also, change the colour for the stop button, let it not be a border but a background of #FF3A46 hex color

@mathiasayivor
Copy link
Collaborator

@Dun-sin does this look good?

server: index.js

socket.on('stop_search', async ({ loginId, email }) => {
        await delWaitingUser(loginId ?? email);
        socket.emit('stop_search_success');
    });

client: BuddyMatcher.jsx

    const handleStopSearch = () => {
        socket.emit('stop_search', { loginId: auth.loginId, email: auth.email });
        setIsStoppingSearch(true);
    };

    useEffect(() => {
        setLoadingText(isStoppingSearch ? stoppingSearchLoadingText : defaultLoadingText);
    }, [isStoppingSearch]);
...
   useEffect(() => {
...
        socket.on('stop_search_success', () => {
            navigate('/');
        });
...
, [])

return isFound ? ... :
{!isStoppingSearch && <button
                onClick={handleStopSearch}
                className={
                    'hover:no-underline hover:text-white font-medium text-white text-[1.5em] w-[8em] h-[2.3em] mt-4 rounded-[30px] border-4 border-solid border-[#f04336] flex flex-col items-center justify-center'
                }
            >
                Stop
            </button>}

@ays14 change the following:

socket.on('stop_search', async ({ loginId, email }) => {
        await delWaitingUser(loginId ?? email);
        socket.emit('stop_search_success');
    });

to:

socket.on('stop_search', async ({ loginId, email }) => {
        await delWaitingUser(email ?? loginId);
        socket.emit('stop_search_success');
    });

@ays14
Copy link
Contributor Author

ays14 commented Oct 26, 2022

@mathiasayivor @Dun-sin done

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

@ays14 did you test this? because it shows stopping search but never does

@Dun-sin
Copy link
Owner

Dun-sin commented Oct 26, 2022

@ays14 sorry never mind, it's not your fault lol

Copy link
Owner

@Dun-sin Dun-sin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good πŸ‘ŒπŸΎ Thank you for contributing your PR is accepted, I hope you continue to contribute to this project

Check out the community discord server πŸ‘‰πŸ½hereπŸ‘ˆπŸ½

@Dun-sin Dun-sin merged commit 10787da into Dun-sin:main Oct 26, 2022
Dun-sin added a commit that referenced this pull request Oct 26, 2022
* feat: added button to stop searching for a buddy (#208)

* feat: added button to stop searching for a buddy

* add changes to remove user from waiting list

* Update BuddyMatcher.jsx

Co-authored-by: ayush-pingsafe <[email protected]>

* fix: mobile display (#205)

* chore: install @rsuite/icons package

* fix: mobile display

Co-authored-by: Ayush Sharma <[email protected]>
Co-authored-by: ayush-pingsafe <[email protected]>
Co-authored-by: Mathias Ayivor <[email protected]>
Dun-sin added a commit that referenced this pull request Oct 26, 2022
* feat: added button to stop searching for a buddy (#208)

* feat: added button to stop searching for a buddy

* add changes to remove user from waiting list

* Update BuddyMatcher.jsx

Co-authored-by: ayush-pingsafe <[email protected]>

* fix: mobile display (#205)

* chore: install @rsuite/icons package

* fix: mobile display

* fix: maintain the same state between navigation (#215)

* fix: update context when stop btn is clicked (#224)

* fix: users can navigate whiles chatting on mobile (#226)

Co-authored-by: Ayush Sharma <[email protected]>
Co-authored-by: ayush-pingsafe <[email protected]>
Co-authored-by: Mathias Ayivor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] add stop button
4 participants