Skip to content
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

Lock on click #66169

Merged
merged 1 commit into from
Jan 15, 2019
Merged

Lock on click #66169

merged 1 commit into from
Jan 15, 2019

Conversation

YisraelV
Copy link
Contributor

@YisraelV YisraelV commented Jan 7, 2019

@isidorn
Copy link
Contributor

isidorn commented Jan 8, 2019

@sandy081 Do you want to review this or should I?
I am fine doing this, but you changed a lot of code in the output land thus I thought you might be a better fit for the review.

@sandy081
Copy link
Member

sandy081 commented Jan 9, 2019

@isidorn Will do.

@sandy081 sandy081 assigned sandy081 and unassigned isidorn Jan 9, 2019
@sandy081 sandy081 self-requested a review January 9, 2019 11:46
@sandy081 sandy081 added the outline Source outline view issues label Jan 9, 2019
@sandy081 sandy081 added this to the December/January 2019 milestone Jan 9, 2019
@@ -612,9 +601,7 @@ export class OutputService extends Disposable implements IOutputService, ITextMo
if (!preserveFocus) {
this._outputPanel.focus();
}
})
// Activate smart scroll when switching back to the output panel
.then(() => this.setPrimaryCursorToLastLine());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason we probably called this function was to resume scrolling. Now we shouldn't call it anymore because scrolling depends only on the lock action state, and the lock state should persist between channel changes.

@YisraelV
Copy link
Contributor Author

YisraelV commented Jan 9, 2019

I added some comments in the code review to clarify my intention. Please let me know if it's preferred not to do it.

@sandy081 I now squashed my two commit into 1 since the second one only fixed something I forgot.

Clicking inside the output panel should set the lock on just as if the
lock icon was clicked. Moving to the last line (with ctrl+end or click)
should unset the lock.
@sandy081
Copy link
Member

@YisraelV I liked toggling the lock button while clicking in the panel.

LGTM

@sandy081 sandy081 merged commit 0e6c4b9 into microsoft:master Jan 15, 2019
@YisraelV
Copy link
Contributor Author

Thanks @sandy081. And also @isidorn and everyone else who's taken time to review my contributions. Really appreciate the experience and confidence I've gained from this project.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outline Source outline view issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants