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

Separate setttings into tabs #176

Merged
merged 5 commits into from Jun 28, 2020

Conversation

ben-kaufman
Copy link
Contributor

Since the Settings page has become quite big and will get bigger in the near future, this PR splits it to General, Auth, and Bitcoin Core settings pages, with tabs like we use for the wallet.

@stepansnigirev
Copy link
Collaborator

Nice, much cleaner now! A few comments...

  1. I think the most important is Bitcoin Core configuration, it should be the first one and open by default when you click on the settings.
  2. Let's rename Auth to Authentication, I think we have enough space for the full word :)
  3. I also think it would be nice to have a link to the HWI bridge settings in the General tab or even iframe it there.

It might make sense to redirect from / to /hwi/settings if specter runs in hwibridge mode, because otherwise it's not clear where to go... But maybe in another PR?

@stepansnigirev
Copy link
Collaborator

Weird behaviour happens when you choose multiuser mode and instead of saving it you click on the OTP link button.
I think OTP button should appear only when you save the authentication.

And maybe it's better not to redirect to the main page from the settings any more, not sure...

Auth -> Authentication.
Add HWI Bridge settigns tab.
Redirect intex page to  HWI Bridge settigns on `--hwibridge` mode.
- Hide Generate new registration link auth not saved as usernamepassword.
- Stop redirect to index after saving settings.
@ben-kaufman
Copy link
Contributor Author

Very strange behavior of the controller test suddenly. When the same code is inside a single test it seems to pass, but when they are in 2 tests I get a 404 suddenly, despite each one separately passing. For now, I just added them together into a single test.

@stepansnigirev
Copy link
Collaborator

stepansnigirev commented Jun 28, 2020

Might be because tests run in random order, so at the end of the test function we need to make sure we are logged in.
At the moment if login logout runs first we are left logged out from the last invalid credentials test.
Can you try moving a valid login test at the end?

@ben-kaufman
Copy link
Contributor Author

This doesn't seem to help :( The problem is that I just get a 404 response error, regardless of which URL I'm trying to query in the second test, as if the client has become inaccessible.

@stepansnigirev stepansnigirev merged commit ad59e8e into cryptoadvance:master Jun 28, 2020
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 this pull request may close these issues.

None yet

2 participants