Keep ranges open #1041#1092
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thanks @Saira-A I just tested in Vercel app and it works fine. 👍 .Well done! |
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @Saira-A -- just a couple quick questions/comments.
| const currentCanvasTopRangeIndex: number = this.getCurrentCanvasTopRangeIndex(); | ||
| const selectedTopRangeIndex: number = this.getSelectedTopRangeIndex(); | ||
| const usingCorrectTree: boolean = | ||
| const currentCanvasTopRangeIndex = this.getCurrentCanvasTopRangeIndex(); |
There was a problem hiding this comment.
The only changes to this particular function seem to be removing types. Just curious if there's a reason for that. If the types were unnecessary because of inference, then I have no objections... I'm not familiar with all the subtleties of when it's best to type or not type -- but the change just stood out so I thought I'd ask. :-)
There was a problem hiding this comment.
Yes, I removed the types because I didn't think the explicit type annotation was necessary but I'm not an expert so I can back to how it was to be just in case :)
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @Saira-A, this looks good to me -- one last comment in case you want to tighten things up a little more, but it doesn't need to stand in the way of approving the PR. :-)
|
|
||
| public getAllNodes(): TreeNode[] { | ||
| return this.treeComponent.getAllNodes(); | ||
| const allNodes = this.treeComponent.getAllNodes(); |
There was a problem hiding this comment.
Did you refactor this (and the method below) for debugging purposes? If so, should they be put back the way they were now that you're done? (If there's a reason to change a one-liner into a two-liner, I don't really mind -- but otherwise it might be better to remain concise).
|
Thanks, @Saira-A and @LanieOkorodudu! Merging now. |
Description of what you did:
Keeps range open when node selected in index view #1041