-
Notifications
You must be signed in to change notification settings - Fork 698
Md list appear #994
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
Md list appear #994
Conversation
expect(wrapper.find('ul')).toHaveLength(1); | ||
expect(wrapper.find(Appear)).toHaveLength(3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to also test for the existence of ListItem
components, but those only appear as you step through the slide. I wasn't sure if figuring out how to imperatively do that was worth the time. The enzyme tests are already in the realm of "testing implementation details", so this set of assertions felt sufficient.
In the future, we might consider moving to react-testing-library
for testing to move away from testing implementation details. My ideal test would be along the lines of:
- Mount the slide.
- Check that none of the list items are present.
- Simulate a click.
- Check that the first list item is present (searching by text), but not the last two.
- Simulate two more clicks.
- Check that all list items are present (searching by text).
import { | ||
CodePane, | ||
Heading, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were some unused imports in this file. I usually clean this up with an editor action for "optimizing imports" (which clears those out). Looks like it also sorts them by alphabetical order, which is why this diff looks bigger than it should. (In effect, all that happened was CodePan
and CodeSpan
were removed.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fantastic!
Description
This PR adds a boolean prop
animateListItems
to theMarkdown
suite of components that automatically transforms MD list items intoAppear > ListItem
. This allows a user to provide MD like:and have the list items appear in one at a time.
This addresses issue #968.
Type of Change
Please delete options that are not relevant (including this descriptive text).
How Has This Been Tested?
I have added some Jest/Enzyme tests to test this new prop. I also added an example to the JS example deck that uses this new prop, which can be used to verify that the list items are animating in accordingly.