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

Setup for Testnet Release #25

Merged
merged 17 commits into from
Dec 8, 2021
Merged

Setup for Testnet Release #25

merged 17 commits into from
Dec 8, 2021

Conversation

bhgomes
Copy link
Contributor

@bhgomes bhgomes commented Dec 1, 2021

@bhgomes bhgomes mentioned this pull request Dec 1, 2021
@bhgomes
Copy link
Contributor Author

bhgomes commented Dec 1, 2021

@Kevingislason Could you take a look at the JS code whenever you have the chance? I believe I have the new state machine working, but there seem to be issues with the UI in general. I'll try and solve them tomorrow as well.

@Kevingislason Kevingislason marked this pull request as ready for review December 7, 2021 06:03
@Kevingislason
Copy link
Contributor

re: closing windows: I retained decorations. If the user tries to close the window on the main page, nothing happens. If the user tries to close the window on the about page, the about window gets hidden.

I tried handling close window requests more intelligently, so that closing the window during authorize declines the transaction. It's less clear how to handle window close during "sign in" and "create account". Suffice it to say, this approach is far more trouble than it's worth. I got declining transactions on close window to work, but sign in was a problem--if you close the Sign in window, when does it reopen? There are some major problems communicating between the service, the rust tauri code, and the front end.

After giving up on the above, I tried getting rid of decorations, but I couldn't figure out how to round the corners of signer's window, which makes it unacceptably ugly.

@bhgomes
Copy link
Contributor Author

bhgomes commented Dec 7, 2021

I tried handling close window requests more intelligently, so that closing the window during authorize declines the transaction. It's less clear how to handle window close during "sign in" and "create account". Suffice it to say, this approach is far more trouble than it's worth. I got declining transactions on close window to work, but sign in was a problem--if you close the Sign in window, when does it reopen? There are some major problems communicating between the service, the rust tauri code, and the front end.

Yeah, I think probably the correct answer is that if they close the window on create account or login it should quit the app entirely. This will require some extra state, so I didn't implement it for now.

After giving up on the above, I tried getting rid of decorations, but I couldn't figure out how to round the corners of signer's window, which makes it unacceptably ugly.

You can make the window transparent (in the Tauri config) and then do a nonzero border-radius in the CSS. Let's keep decorations for now though.

Comment on lines +316 to +319
#[cfg(debug_assertions)]
let url = config.dev_origin_url.as_str();
#[cfg(not(debug_assertions))]
let url = config.prod_origin_url.as_str();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have anything against us allowing both CORS urls always? It is a bit inconvenient to have to manage separate prod and dev builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want the localhost allowed on the production build. That's a super easy way to spoof the frontend with a trojan and we should also not test some production parts with non-production parts. I just wish there was a better way to structure this instead of relying on debug_assertions which is a bit sketchy but technically safe.

@Kevingislason Kevingislason self-requested a review December 8, 2021 08:44
Copy link
Contributor

@Kevingislason Kevingislason left a comment

Choose a reason for hiding this comment

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

Ran the tests on deployed version of testnet UI, everything working

LGTM

@bhgomes bhgomes merged commit cb02ddb into master Dec 8, 2021
@bhgomes bhgomes deleted the setup-release branch December 8, 2021 21:52
@bhgomes bhgomes added the L-added Changelog: add these changes to the `added` section of the changelog label Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L-added Changelog: add these changes to the `added` section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't quit sometimes Handle close window Handle incorrect password
2 participants