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 on-complete event instead of on-error and on-success in keycard flow #22166

Open
Parveshdhull opened this issue Feb 20, 2025 · 3 comments

Comments

@Parveshdhull
Copy link
Member

Parveshdhull commented Feb 20, 2025

Problem

In keycard flows and events, such as keycard/connect and :keycard/get-application-info, we are currently passing two props: on-success and on-error. I believe we should migrate to a single prop, on-complete, which includes an error parameter. Why?

  • We are checking for errors in on-error and conditionally performing positive actions based on that in on-error, which seems confusing. Examples can be found here: Link 1 and Link 2.

  • Sometimes, we need to perform a positive action in both the on-success and on-error cases. In such scenarios, we are currently passing both parameters, where on-success directly performs the positive action, while on-error conditionally performs the same action. Instead, we can pass a single parameter in these cases.

Please feel free to suggest better name for the parameter instead of on-complete

@Parveshdhull
Copy link
Member Author

on-complete, which includes an error parameter.

Maybe instead of error parameter, we can name it validation-error. Because it was not error returned by keycard/connect or :keycard/get-application-info. We are just validating returned data and returning error value based on that validation. Keycard connect worked fine.

@Parveshdhull
Copy link
Member Author

cc @flexsurfer

@flexsurfer flexsurfer self-assigned this Feb 20, 2025
@flexsurfer
Copy link
Member

thanks @Parveshdhull for the issue, im working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

2 participants