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

SEP-24 popup window closes immediately #9

Open
ElliotFriend opened this issue Sep 24, 2024 · 11 comments · May be fixed by #28
Open

SEP-24 popup window closes immediately #9

ElliotFriend opened this issue Sep 24, 2024 · 11 comments · May be fixed by #28
Assignees
Labels
bug Something isn't working good first issue Good for newcomers ODHack8 Issues eligible for the OnlyDust hackathon

Comments

@ElliotFriend
Copy link
Contributor

When a user clicks the SEP-24 deposit/withdraw buttons on the /dashboard/transfers page, the popup displays for a moment, and immediately closes. I suspect the window is picking up a stream of events in the eventListener part now.

We should add some kind of testing logic that only does popup.close() in the event that the postMessage says something like "transfer initiated" or "successful" or "aborted" or whatever the status messages might be.

@ElliotFriend ElliotFriend added bug Something isn't working ODHack8 Issues eligible for the OnlyDust hackathon good first issue Good for newcomers and removed good first issue Good for newcomers labels Sep 24, 2024
@ShantelPeters
Copy link
Contributor

I will love to work on this @ElliotFriend kindly assign

Copy link

onlydustapp bot commented Sep 25, 2024

Hi @ShantelPeters!
Maintainers during the ODHack # 8.0 will be tracking applications via OnlyDust.
Therefore, in order for you to have a chance at being assigned to this issue, please apply directly here, or else your application may not be considered.

@martinvibes
Copy link

i would love to work on this issue @ElliotFriend please assign me

Copy link

onlydustapp bot commented Sep 25, 2024

Hi @martinvibes!
Maintainers during the ODHack # 8.0 will be tracking applications via OnlyDust.
Therefore, in order for you to have a chance at being assigned to this issue, please apply directly here, or else your application may not be considered.

@Dprof-in-tech
Copy link

Dprof-in-tech commented Sep 26, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello, i am Dprof-in-tech, an experienced Full Stack Blockchain Developer and I am excited to contribute my skills to this project in this ODHACK 8. With a strong background in Next.js, TypeScript, JavaScript, React, Node.js, Rust and Cairo, I've honed my technical skills across the blockchain development landscape.

My journey with OnlyDust began at Edition 2, and I've since made 34 contributions across 11 projects. This extensive experience on the platform has allowed me to develop a keen understanding of delivering high-quality solutions under tight deadlines.
Due to my web2 experience, i am skilled in writing unit tests and have used Jest and Vitest to do this. I would love to contribute to this project by solving this issue.

Below is a link to my OnlyDust public profile.
https://app.onlydust.com/u/Dprof-in-tech

How I plan on tackling this issue

Based on the code provided and the issue described, here's an approach to solve the problem of the SEP-24 deposit/withdraw popup closing prematurely:

  1. Identify the Issue:
    The problem occurs in the launchTransferWindowSep24 function, where the event listener is closing the popup immediately upon receiving any message.

  2. Modify the Event Listener: i would modify the event listener to decode the message being returned before closing the pop up.

  3. Implement Error Handling:

    • Add try-catch blocks to handle potential errors in the event processing.
    • Log errors for debugging purposes.
  4. Potentially Add User Feedback if needed:

    • Implement a mechanism to display status updates to the user.
    • Consider using a toast notification system or updating a status element on the page.
  5. Testing:

    • Manually test the SEP-24 flow with various scenarios to ensure the popup stays open until the transfer is completed or explicitly aborted.
    • Add unit tests for the new event handling logic.
  6. Refactor for Reusability:

    • Consider extracting the SEP-24 window handling logic into a separate function or component for easier maintenance and potential reuse.
  7. Update Documentation where neccessary:

    • Add comments explaining the new behavior of the event listener.
    • Update any relevant documentation about the SEP-24 transfer process.

@0xdevcollins
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

My name is Collins Ikechukwu. I'm a full stack blockchain developer developer.

How I plan on tackling this issue

  1. Modify Event Listener: Update the event listener to check for specific status messages before calling popup.close().

  2. Define Accepted Statuses: Create an array or set of acceptable status messages, such as "transfer initiated," "successful," and "aborted."

  3. Conditional Logic: Implement logic that only invokes popup.close() if the received message matches one of the accepted statuses.

  4. Example Implementation:

    const acceptedStatuses = ["transfer initiated", "successful", "aborted"];
    
    window.addEventListener("message", (event) => {
        if (acceptedStatuses.includes(event.data.status)) {
            popup.close();
        }
    });
  5. Testing: Test the functionality to ensure that the popup only closes for the specified status messages, avoiding unintentional closures.

@Solomonsolomonsolomon
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Fullstack developer

How I plan on tackling this issue

This is a simple bug,I will inspect the code base and add logic to only close the popup after the post message

@Benjtalkshow
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

My name is Benjamin, and as a full-stack JavaScript developer with experience in creating responsive and user-friendly components, I understand the importance of a smooth user experience. I have made 18 contributions to the OnlyDust project and am confident in my ability to resolve the current issue with the SEP-24 popup.

How I plan on tackling this issue

The current problem is that when users click the deposit or withdraw buttons, the popup appears briefly and then closes right away. This can be frustrating for users, as they don't have enough time to read the important transaction status messages.

To fix this, I suggest we update the event listener that handles the messages sent by the popup. We should make sure that the popup only closes when it receives specific messages, such as “transfer initiated,” “successful,” or “aborted.” This way, the popup can stay open longer for any messages that don’t indicate it should close.

I'll also conduct thorough testing to ensure the popup behaves correctly with different status messages, giving users ample time to read them.

By making these changes, we can significantly improve the user experience and ensure that important transaction information is communicated clearly. I'm eager to hear your thoughts and get started on this solution.

@martinvibes
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

hello i am a frontend dev and blockchain developer
please can i work on this issue :) and would love to be a contributor

How I plan on tackling this issue

Proposed Solution:
Improve Event Listener Logic:
Modify the event listener to close the popup only when the postMessage event contains a valid status, such as:
"transfer initiated"
"successful"
"aborted"
Prevent Premature Closing:
Ensure the popup doesn’t close for any other messages or events, preventing unintended behavior.

Possible Cause:
The event listener is reacting to irrelevant events, causing the popup to close before a valid status is received.

@t0fudev
Copy link

t0fudev commented Sep 27, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Helloo, my name is Hellen. I´m currently a computer science student and a web3 developer. I have been working on different exercises on different platforms like Exercism and Starklings in the past weeks so I can get my knowledge about the different languages better . I am hopeful to contribute on this repository!!

How I plan on tackling this issue

To fix the issue where the popup closes immediately when users click on the button is to modify the event listener by implementing a check within the listener using "handlePopupMessage" so it closes to a specific message. (In this case to "transfer initiated", "successful", or "aborted".)

@josephchimebuka
Copy link
Contributor

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello @ElliotFriend I am Joseph I am a Software developer and blockchain developer and I am also an active contributor here on only dust here is my profile https://app.onlydust.com/u/josephchimebuka. This is my first time to contribute to this repo ill appreciate the opportunity to contribute.

How I plan on tackling this issue

To fix this, I will add logic to the event listener that checks the postMessage content before triggering popup.close(). I'll ensure that the popup only closes when the message contains specific status values like "transfer initiated", "successful", or "aborted". This will prevent the popup from closing prematurely by only reacting to valid status updates. After making the change, I will test the functionality to ensure the popup behaves correctly based on the messages received.

@t0fudev t0fudev linked a pull request Oct 4, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers ODHack8 Issues eligible for the OnlyDust hackathon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants