Remove History menu option for the tryit notebook (fixes #2114) #2125
Conversation
There was a problem hiding this comment.
This looks good minus the one requested change I have. Could you also add an entry to CHANGELOG.md per the pull request instructions?
Also, generally we add a "(fixes #xxx)" to end in a PR/commit message when a PR fixes an existing issue. If you could add/change that, that would be great too :)
| }; | ||
|
|
||
| render() { | ||
| return ( | ||
| <NotebookIconMenu> | ||
| {this.props.isServer && <NotebookMenuItem task={tasks.newNotebook} />} | ||
| {this.props.isServer && ( | ||
| {!this.props.isTrialNotebook && ( | ||
| <NotebookMenuItem task={tasks.toggleHistoryModal} /> |
There was a problem hiding this comment.
I think I'd prefer to disable the item in this case, since there is a way to get access to the functionality (log in). Try this instead:
<NotebookMenuItem task={tasks.toggleHistoryModal} disabled={this.props.isTrialNotebook} />There was a problem hiding this comment.
Thanks, good point! Total React n00b - it seems the above code won't work either since the item doesn't get rerendered on login? I'll try to figure out how this actually works.
There was a problem hiding this comment.
I think the reason is that the notebook tryit mode boolean isn't set to false after login and notebook creation - PR updated, hope this works!
|
@wlach thanks for the review! Saw the checklist after submitting - will fix now. |
ZiyaoWei
changed the title
|
Works great, thank you! |
Addresses #2114 by not showing the History option for the tryit notebook.
Pull Request checklist