feat(VerticalNav): Add a prop to allow disabling persistence - #307
Conversation
Pull Request Test Coverage Report for Build 1206
💛 - Coveralls |
7962cd1 to
c014c55
Compare
| expect(component.render()).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| test('VerticalNav renders properly with persistence off', () => { |
There was a problem hiding this comment.
I don't feel like this test does much. To rest the persist ternary you could export WithPersist and NoPersist from VericalNav.js (not the index.js) and then import them in the test. You could then assert similar to below:
const component = shallow(<VerticalNav persist />);
expect(component.find(WithPersist).exists()).toBe(true);and
const component = shallow(<VerticalNav persist={false} />);
expect(component.find(NoPersist).exists()).toBe(true);This still doesn't cover everything, but it probably is not worth writing tests for the controlled HOC if it is going away.
There was a problem hiding this comment.
That makes sense. I mostly copypasted as a sanity check to make sure the component doesn't crash when rendered that way. I like your version better. You're full of little snippets for my code library @dmiller9911.
You can now pass persist={false} to VerticalNav to turn off the persistence behavior.
c014c55 to
f501f75
Compare
|
@jeff-phillips-18 I rebased this and folded in @dmiller9911's feedback, can you re-approve when you get a chance? |
|
i'm ok w/ this for now - knowing that we are revisiting |
5 participants
What:
You can now pass persist={false} to VerticalNav to turn off the persistence behavior.
Link to Storybook:
http://rawgit.com/mturley/patternfly-react/vert-nav-persist-sb/index.html