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

Add progress indicator when fetching a studio token #5000

Merged
merged 6 commits into from
Nov 27, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Nov 15, 2023

Demo

Screen.Recording.2023-11-17.at.7.33.12.AM.mov

Part of #3864

@julieg18 julieg18 added the product PR that affects product label Nov 15, 2023
@julieg18 julieg18 self-assigned this Nov 15, 2023
@julieg18 julieg18 changed the title Add loading message to studio setup section when fetching a studio token Add progress indicator when fetching a studio token Nov 17, 2023
const token = await this.fetchStudioToken(deviceCode, tokenUri)
const cwd = this.getCwd()
private requestStudioToken(deviceCode: string, tokenUri: string) {
return Toast.showProgress('Connecting to Studio', async progress => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally, I was going to update our Studio section with a loading message but, after a 1v1 with Stephanie, decided on using a progress Toast instead since:

  • The process is practically near instant for someone with a fast computer, it was just looking really slow from my end since my computer is on the slower side.
  • A Toast is less code to do compared to needing to send a message back and forth to the webview.

@@ -631,7 +631,7 @@ describe('App', () => {
})

describe('Studio not connected', () => {
it('should show buttons which request a token from Studio or add an already creatd one', () => {
it('should show buttons which request a token from Studio or add an already created one', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spotted a typo that I went ahead and fixed.

Copy link
Member

Choose a reason for hiding this comment

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

@@ -7,7 +7,7 @@ import { Button } from '../../../shared/components/button/Button'
export const Connect: React.FC = () => {
return (
<EmptyState isFullScreen={false}>
<div data-testid="setup-studio-content">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty unrelated to this pr since I'm no longer updating this section, but we're not using the test id anywhere.

@julieg18 julieg18 marked this pull request as ready for review November 17, 2023 14:13
@julieg18 julieg18 enabled auto-merge (squash) November 27, 2023 13:40
Copy link

codeclimate bot commented Nov 27, 2023

Code Climate has analyzed commit 2b87439 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.2% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit 985b224 into main Nov 27, 2023
8 checks passed
@julieg18 julieg18 deleted the add-loading-screen-to-studio-auth branch November 27, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants