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

Enable access to isLoading true state #13

Merged
merged 3 commits into from
Jul 27, 2020
Merged

Conversation

stevehobbsdev
Copy link
Contributor

@stevehobbsdev stevehobbsdev commented Jul 22, 2020

This PR changes the isLoading$ observable emission behaviour to allow access to when the value is true. This is useful for the UI so that it can display some kind of loading spinner or text before the rest of the UI is available.

This PR also does some refactoring of the tests to include a couple of helpers, but also moves the setup of the service into the it blocks so that its creation can be controlled a bit more carefully.

expect(auth0Client.handleRedirectCallback).toHaveBeenCalledTimes(1);
done();
});
});

it('should redirect to the correct route', (done) => {
const service = TestBed.inject(AuthService);
const service = createService();
Copy link
Contributor

Choose a reason for hiding this comment

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

lots and lots of createService(). Any reason to avoid a describe-scoped or even file-scoped beforeEach()?

describe('when some case', () => {
    beforeEach(() => {
      createService();
    });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, let me revisit this. The motivation for moving it was so that the exact point of creation could be determined as a few tests need to set up something before the service is created (testing functionality that happens in the constructor), but perhaps some of that is not needed any more.

Let me clean it up a bit.

@stevehobbsdev stevehobbsdev merged commit 93c6daf into master Jul 27, 2020
@stevehobbsdev stevehobbsdev deleted the fix/is-loading branch July 27, 2020 10:25
@lbalmaceda lbalmaceda added this to the v0.1.0 milestone Aug 14, 2020
@lbalmaceda lbalmaceda added the CH: Added PR is adding feature or functionality label Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added PR is adding feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants