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

Implement the SEP24 flow in an iframe #102

Open
prayagd opened this issue Aug 14, 2024 · 31 comments
Open

Implement the SEP24 flow in an iframe #102

prayagd opened this issue Aug 14, 2024 · 31 comments
Assignees

Comments

@prayagd
Copy link
Collaborator

prayagd commented Aug 14, 2024

Context

Currently the MYKOBO flow is implemented as redirect to other tab when user clicks on "Start offramping", Instead of that it should be iframe on the same screen.

Acceptance Criteria

  • On the Vortex dApp after the user agrees and clicks on "Start offramping"
  • The MYKOBO page should open-up as a iframe and user should be complete the steps from that iframe
  • Show the input summary card with all the details shown in the design attached above the iframe
  • If its the first time dApp user, then it should start from the KYC page.
    • With the following title "Verify your identity"
    • With the following text above the iframe "Please follow the steps below to complete your identity verification."
  • If the user is already KYC'ed, they should land on the banking details page.
    • With the following title "Enter bank details"
    • With the following text above the iframe "Please do not change the amount on offramp UI"
  • The text and title should change when the first time KYC user transitions to banking details page
  • After completing the steps on the MYKOBO UI, the iframe should close and the vortex dAPP should start the next steps i.e signing of the polygon txn automatically. This should happen immediately.

Designs

https://www.figma.com/proto/cOx99YQtxf8LLdlhZjvkoC/Vortex?page-id=330%3A556&node-id=330-557&node-type=FRAME&viewport=-2578%2C-399%2C0.34&t=VO6VI0x4i1oMtmwu-1&scaling=min-zoom&content-scaling=fixed&starting-point-node-id=330%3A557

@prayagd prayagd added the draft label Aug 14, 2024
@prayagd
Copy link
Collaborator Author

prayagd commented Aug 14, 2024

@pendulum-chain/devs Before writing this ticket i want to confirm if something of this level is possible or not, also Note - this is not a feature for the current version but to be maintained as backlog ticket for future version

@gianfra-t
Copy link
Contributor

IIRC we had some issues with the iframe regarding detection of the user's submission. As I understand it is not easy (or possible?) to communicate and read the content of an iframe with a different URL, therefore I don't know how it would be possible to close the iframe once the user is done with the KYC.

@Sharqiewicz you may have more insights on this perhaps.

@ebma
Copy link
Member

ebma commented Aug 26, 2024

It's possible and we did this in the first iterations of the prototype IIRC. Regarding the callback for the KYC, you might be right that it's not easy but in the worst case we just show a button above or below the iframe that the user can click to close the iframe again.

@TorstenStueber
Copy link
Member

It should not be a problem to close the iframe. We are currently repeatedly querying the Mykobo API whether the SEP24 flow has been completed and only then continue the flow. We can then just close the iframe once we detect this.

I think that the only reason why the iframe solution might not be feasible is if Mykobo explicitly disallows that their website is embedded (e.g., through this or this). Something we can easily test.

@prayagd
Copy link
Collaborator Author

prayagd commented Aug 28, 2024

@pendulum-chain/devs Do we need design for this? I just wrote a basic user story based on how it should work

@prayagd
Copy link
Collaborator Author

prayagd commented Aug 28, 2024

Let me know if anything is missing or anymore tech details are to be added.

@prayagd
Copy link
Collaborator Author

prayagd commented Aug 28, 2024

Hey team! Please add your planning poker estimate with Zenhub @bogdanS98 @ebma @gianfra-t @TorstenStueber

@prayagd prayagd removed the draft label Aug 28, 2024
@ebma
Copy link
Member

ebma commented Aug 28, 2024

Yes, some design would be nice. Doesn't have to be perfect but just so we know where to place the iframe and in what size etc.

@prayagd
Copy link
Collaborator Author

prayagd commented Aug 28, 2024

cc @Klausdmz can we get a basic design of this, you can find the user story above in the main description. Let me know if any questions we can pick it up in the design sync.

@vadaynujra vadaynujra changed the title Implement the MYKOBO flow in a iframe Implement the MYKOBO flow in an iframe Aug 28, 2024
@TorstenStueber
Copy link
Member

Does this really need a design? We just need to know

  • where to place the iframe (e.g., under the form)
  • the width of the iframe (e.g., the width of the screen or the width of the form)
  • the fixed height

@Klausdmz
Copy link
Collaborator

@prayagd Here are two screens showing the iframe design: Screen 1 and screen 2.

The recommended size is 535 x 550 pixels, as it aligns with the source content and fits well within the layout, including the title and message above. For mobile devices, it should be proportionally scaled down to prevent horizontal scrolling and ensure it fits within the screen. This helps avoid nested scrolling, where the user has to scroll both on the main page and within the iframe.

@ebma
Copy link
Member

ebma commented Aug 29, 2024

Looking at the screenshots, it seems like it's a separate/new screen replacing the existing form with its input fields. Is this intended? If the form is replaced by that iframe, what should happen once the KYC is done?

@TorstenStueber
Copy link
Member

I wonder whether it is best UX to switch to a new screen. On the one hand it is cleaner that way but on the other hand the user loses all context about the input value and expected exchange rate.

@ebma It makes sense to just ping the anchor API to recognize whether the flow has been completed (that's what we already do). Once completed we could then switch to the progress screen. However, this would also require to move the signing progress widget introduced in this issue to the progress screen (right now it is still on the initial screen).

@Klausdmz
Copy link
Collaborator

If we want to keep the existing form with the input fields visible alongside the KYC iframe on the same screen, it would need to be positioned below the form. This setup would require users to scroll down to the bottom of the page to access the iframe on mobile, which isn’t very intuitive. Perhaps a compact summary card with key information could be a better solution? Here's an idea:
Screenshot 2024-08-30 at 12 13 27

@TorstenStueber
Copy link
Member

That sounds alright to me, we would need to communicate again that there are two different output values, similar to the discussion in this ticket.

@gianfra-t gianfra-t self-assigned this Aug 30, 2024
@gianfra-t
Copy link
Contributor

gianfra-t commented Sep 2, 2024

@pendulum-chain/product I see that this is still in being prepared, but besides the iframe that is a certainty we want, do we also want to display this compact version the summary card @Klausdmz shared in the previous message? Or is this still in discussion.

@prayagd
Copy link
Collaborator Author

prayagd commented Sep 3, 2024

@gianfra-t we are still finalising the designs, to include a summary card of the input field form and also the message for the user to show that please dont change the amount on the anchor screen.

@prayagd
Copy link
Collaborator Author

prayagd commented Sep 3, 2024

@Klausdmz Please share the new design here we discussed today,

  • Message for user to not change the amount on the anchor screen
  • Also a summary card showing the details entered by the user on the input form

@Klausdmz
Copy link
Collaborator

Klausdmz commented Sep 3, 2024

Just updated the summary card and iframe design here and here. The iframe height was reduced from 550 to 450 px, and I highlighted the output amount due to its relevance at this step. I also added instructions to enter that amount in the field. What do you think?

@TorstenStueber
Copy link
Member

What is the reasoning to reduce the height of the iframe? Is it bad if it is taller? There is not way to predict the height of screen of the user and there will be many cases where the iframe does not fit as ideally as on the designs.

A smaller iframe makes does not necessarily improve the UX, it feels more like looking at the content through a peephole (my opinion).

@prayagd
Copy link
Collaborator Author

prayagd commented Sep 4, 2024

What is the reasoning to reduce the height of the iframe? Is it bad if it is taller?

I assume it is to fit the summary card and the text shown above, as it is important for the user to know what the input details with a summary card. correct me if wrong cc @Klausdmz

We can improve the screen iteratively, even if the iframe becomes smaller the user can navigate within the iframe using the scroll. So IMO its good to go. Will update the main ticket description.

@prayagd
Copy link
Collaborator Author

prayagd commented Sep 4, 2024

@pendulum-chain/devs ticket description updated and design too

@Klausdmz
Copy link
Collaborator

Klausdmz commented Sep 4, 2024

@TorstenStueber The height reduction is intended to avoid nested vertical scrolling, which can negatively impact the UX, particularly on mobile. Initially, I specified the height in px based on a particular screen size, but for a responsive design, it’s better to define the height as a percentage of the screen height. In both desktop and mobile views, this would correspond to 50% of it. I’ve also updated the summary card, now emphasizing the quote instead of the amount to receive.

@Sharqiewicz
Copy link
Contributor

Communication with the iframe is limited for several reasons.

  • Primarily, SOP/CORS policies prevent interaction unless the internal page is explicitly configured to allow it. As a result, communication is typically limited to the shared API provided by the embedded site (in our case SEP-24).
  • Regarding adjusting the iframe’s height based on its content, this is not possible because we do not have the ability to modify the code of the iframe’s source. If the iframe source were to send messages via the window.postMessage API, we could dynamically adjust the height, but it doesn’t. We can only adjust the height by height property in <iframe>

@TorstenStueber
Copy link
Member

I will move this out of icebox as Mykobo is working on making the iframe solution possible.

@vadaynujra
Copy link

vadaynujra commented Sep 23, 2024

What is expected from them and have they shared any timeline by when they plan to complete?

@TorstenStueber
Copy link
Member

We should remove the gas pump symbol from the design. We removed it also from the most recent version of the fee details box on the main screen. The fees are not gas fees but anchor fees.

@vadaynujra
Copy link

vadaynujra commented Sep 24, 2024

Makes sense @TorstenStueber. @prayagd could you create a ticket for this and check what would be a better alternate symbol?

@prayagd
Copy link
Collaborator Author

prayagd commented Sep 25, 2024

#175 here is the ticket, tried to find the alternative for offramp icon but i guess there i no standard icon for it. Should i ask @Klausdmz to make a new one?

Based on the slack message about this ticket, there was a conclusion that we are not going to proceed with this solution. Should i move it to icebox?

@TorstenStueber
Copy link
Member

I did not mean to create a new ticket but just to change the specification of this ticket here instead.

Anyway, we can icebox it for now

@vadaynujra vadaynujra changed the title Implement the MYKOBO flow in an iframe Implement the SEP24 flow in an iframe Sep 26, 2024
@vadaynujra
Copy link

Updated the title replacing 'MYKOBO' to 'SEP24' for when we next look at it more broadly for the SEP24 flow. Moving to Icebox.

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 a pull request may close this issue.

7 participants