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

Adds Tests for <Loading>'s Timer + Minor Improvements #210

Merged
merged 3 commits into from
Feb 22, 2020

Conversation

mattxwang
Copy link
Member

@mattxwang mattxwang commented Feb 21, 2020

This is a quick follow up to @rzlien's PR (#197) adding a help text timer to <Loading>. It does three things:

  1. renames the prop showText to showHelpText, and makes it inherit the value from an optional prop
  2. uses GH_REPO_NAME constant instead of hard link for repo
  3. adds tests to check for showHelpText prop validity and to check that the timer works (i.e. on mount, the help text is not there, and then after 2s, the help text shows up)

Not super happy with how the timer test is written, since I wanted to use Jest's mock timer calls - but I wasn't able to get them to work.

don't think I implemented this in the best way, but using Jest's mock timers didn't work as intended. not entirely sure why :(
@mattxwang mattxwang added the refactor Refactoring & cleanup label Feb 21, 2020
Copy link
Contributor

@krashanoff krashanoff left a comment

Choose a reason for hiding this comment

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

Hey Matt! I'm digging the refactors that have been going on lately -- definitely good to be cleaning up some of the code. Visually everything seems to be in order, but the test isn't passing because the snapshot is missing two details.

Copy link
Contributor

@krashanoff krashanoff left a comment

Choose a reason for hiding this comment

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

Scratch that. Needed to update some packages before running the same tests. My mistake, LGTM.

@mattxwang mattxwang merged commit 58bc2ad into master Feb 22, 2020
@mattxwang mattxwang deleted the loading-component-improvements branch February 22, 2020 23:26
@mattxwang mattxwang restored the loading-component-improvements branch February 29, 2020 01:33
@mattxwang mattxwang deleted the loading-component-improvements branch February 29, 2020 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring & cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants