Make result name suggestion for new msg waits #181
Conversation
tsconfig.json
Outdated
| @@ -14,7 +14,7 @@ | |||
| "allowJs": true, | |||
| "watch": true, | |||
| "alwaysStrict": true, | |||
| "lib": ["es6", "dom"] | |||
| "lib": ["es2017", "dom"] | |||
There was a problem hiding this comment.
Object.values() .. so certainly not necessary, but don't see any reason to avoid it.
More here: microsoft/TypeScript#10768
There was a problem hiding this comment.
Hmmm.. but that is failing the node 6 build, so maybe there's one.
There was a problem hiding this comment.
nice, can't wait for that to land
src/component/Flow.tsx
Outdated
| @@ -330,9 +340,30 @@ export default class Flow extends React.Component<FlowProps, FlowState> { | |||
|
|
|||
| ghost.actions.push(replyAction); | |||
| } else { | |||
| // coun't up all our msg waits | |||
| let responseNumber = 1; | |||
There was a problem hiding this comment.
should these be broken-out into helper methods, e.g. getResponseNumber and getResultName? would that make testing easier?
| this.setState({ | ||
| show: true, | ||
| config | ||
| ...this.getInitialState(), |
There was a problem hiding this comment.
yes eric, yes...embrace the spread operator...
…nto named_msg_waits_156
|
@ycleptkellan what say we merge this so we don't have anything outstanding against master |
|
aye, merge away |
| * Gets a suggested result name based on the current number of waits | ||
| * in the current definition | ||
| */ | ||
| public getSuggestedResultName(): string { |
There was a problem hiding this comment.
@ericnewcomer what think you about–in the future–shying away from adding methods to component classes that don't either handle a callback that changes state somewhere in the render tree or determine whether or not markdown is rendered? my reasoning is that a method that doesn't read from the component's this context is much easier to test (no need to render Flow and access it's instance before testing getSuggestedResultName).
| @@ -40,5 +40,16 @@ describe('Flow >', () => { | |||
| expect(getSpecWrapper(wrapper, 'nodes').hasClass('node_list')).toBeTruthy(); | |||
| expect(wrapper.find('NodeComp')).toBeTruthy(); | |||
| }); | |||
|
|
|||
| it('should suggest appropriate response names', () => { | |||
| const wrapper = shallow(<Flow {...props} />, { | |||
release: 2.10.0
Fixes: #156 (this still needs unit tests)