-
Notifications
You must be signed in to change notification settings - Fork 51
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
[CircleGraph] Implement visqselector mode #1545
Conversation
/** | ||
* @brief toggleSelect will select or toggle select with CtrlKey down | ||
* @param viewNode view.Node instance | ||
* @note works on host mode is viewMode.selector | ||
*/ | ||
toggleSelect(viewNode) { | ||
if (viewNode && this._host._mode === viewMode.selector) { | ||
if ( |
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.
(optional) How about returning 'if not' condition here to reduce the level of condition?
Like,
if (!viewMode) {return}
if (this._host._mode === viewMode.selector || this._host._mode === viewMode.visqselector) {return}
// continuous logic
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.
@dayo09
Thank you. Unfortunately there are 4 values, including visqselector
, so inverting logic will still produce:
if (this._host._mode !== viewMode.viewer && this._host._mode !== viewMode.visq) {return}
// continuous logic
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 mean, if it's not 'viewMode', then this function don't need to proceed. Note that it's an 'and' condition. 😄
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.
Ahhh. Sorry. I didn't immediately understand that.
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.
@dayo09
fixed.
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.
LGTM
753a5c3
to
37ad016
Compare
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.
Thanks!
37ad016
to
21c06c3
Compare
This commit implements visqselector mode and managing of scrollToSlected. ONE-vscode-DCO-1.0-Signed-off-by: s.malakhov <s.malakhov@partner.samsung.com>
21c06c3
to
c5c37c9
Compare
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.
LGTM
This commit implements visqselector mode and managing of scrollToSlected.
Its correctness is tested in #1543
Fresh draft: #1543
Full draft: #1505
Related: #1491
ONE-vscode-DCO-1.0-Signed-off-by: s.malakhov s.malakhov@partner.samsung.com