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

Replace alert in login that use LError with gi.ui/prompt #2070

Open
taoeffect opened this issue Jun 14, 2024 · 2 comments
Open

Replace alert in login that use LError with gi.ui/prompt #2070

taoeffect opened this issue Jun 14, 2024 · 2 comments

Comments

@taoeffect
Copy link
Member

taoeffect commented Jun 14, 2024

Problem

In #2069 I got this error:

tmp-1718390238728

This is clearly a problematic prompt as it shows HTML to the user instead of rendering it:

      console.error('gi.actions/identity/login failed!', e)
      const humanErr = L('Failed to login: {reportError}', LError(e))
      alert(humanErr)

EDIT: I just noticed this is a more widespread problem in the codebase, as this use of alert seems to exist in the contracts too:

tmp-1718396956352

IMPORTANT: the use of alert by itself isn't a problem, but when the message contains HTML - gi.ui/prompt must be used instead because only that can render it.

Solution

  1. Replace the call to alert with gi.ui/prompt
  2. Test by throwing an error during login and verify that the link is rendered correctly

Make 100% sure to test every alert that's replaced with gi.ui/prompt to verify it displays correctly.

If called from contracts, gi.ui/prompt will need to be whitelisted in main.js under allowedSelectors.

@taoeffect taoeffect changed the title Replace alerts in login that use LError with gi.ui/prompt Replace alert in login that use LError with gi.ui/prompt Jun 14, 2024
@SebinSong SebinSong self-assigned this Jun 14, 2024
@dotmacro
Copy link
Member

I just noticed this is a more widespread problem in the codebase

Is the screenshot in #2051 another example?

@taoeffect
Copy link
Member Author

Is the screenshot in #2051 another example?

Yeah that could be another thing to boy-scout for this issue 👍

taoeffect pushed a commit that referenced this issue Jun 28, 2024
* replace one alert() to in-app prompt in identity contract

* replace more alert() in the login-flow with throw new GIErrorUIRuntimeError()

* make sure the prompt replaces alert() for error joining the general chatroom

* replace alert() from some more places

* error prompt for group-joinning error

* remove random error thrown on purpose

* update PR according to the feedbacks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants