Show errors more prominently and show token source #228
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johnstcn
approved these changes
Apr 24, 2023
6f0024b to
aa54370
Compare
Should make it easier to test. This meant not calling askTokenAndConnect because it interacts with the global model. Initially I was trying to figure out a way to wait progress dialog background job so you could then retry but I am not sure how to do that so I went with passing in a callback. Also launching a new coroutine since otherwise it blocked the current job and threw a warning, I think it disliked the invokeAndWait of the token dialog from inside the job.
Instead of just "Nothing to show" it will show the last error or the status if we are currently trying to connect.
Now it will say whether the token was from the config or was the last known token and if it fails there will be an error message. You could always check the error in the bottom right but this way it is more obvious why the token dialog has reappeared. Also if the URL has changed there is no point trying to use the token we had stored for the previous URL.
a11ae83 to
e3bf092
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is the little bubble in the lower right corner but I think showing the error next to the field or in the table can help identify problems more quickly and in cases where we prompt for a token again I think it is a bit non-obvious at first why it suddenly appears when there is no accompanying text.
Also hoping it will help in cases like #226 where "nothing to show" could mean the workspaces list response was empty or it could mean any number of things went wrong while trying to connect.
As I was doing this I made the token nullable which meant I would have to add a null check in
connectas it pulled from the local model even though technically it could never be null (we always set it before callingconnect) so I refactored it to not rely on the local model, you just pass things in now.