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

Keep top line in view when resizing Console to avoid losing user context #13695

Merged
merged 3 commits into from
Aug 25, 2017

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Aug 24, 2017

fixes #10677

When moving the divider pane in console, the editor will keep the cursor in the center. If no line is selected, then the editor will keep the top line at the start of the dragging as the top line.

after

There is nothing in ace.js that natively does this. There is an open issue, ajaxorg/ace#3339. This PR just keeps track of the cursor or top row when dragging starts and then scrolls to that row after calling resize.

@kobelb
Copy link
Contributor

kobelb commented Aug 25, 2017

Functionally, this works well until I have results in my output, and then things start to behave weirdly.

devtools-resizing

Regarding the technical implementation, I find it somewhat confusing that we're storing the peggedRow on the editor itself, since we're only really using it for the resize operation. If we were to change the smartResize to have a property like topRow we could potentially set this when the resize mousedown event occurs and then use this when determining whether to scroll or not.

@nreese
Copy link
Contributor Author

nreese commented Aug 25, 2017

@kobelb Thanks for the good idea of moving the state to smart_resize. This clears up the weirdness in the output since now both the output and the input will keep the top line in view when resized. I also removed the moving to cursor as suggested.

Copy link
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

LGTM

@nreese nreese merged commit 5619dbe into elastic:master Aug 25, 2017
nreese added a commit to nreese/kibana that referenced this pull request Aug 25, 2017
…d losing user context (elastic#13695)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
@nreese nreese added the v5.6.0 label Aug 25, 2017
nreese added a commit to nreese/kibana that referenced this pull request Aug 25, 2017
…d losing user context (elastic#13695)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
nreese added a commit to nreese/kibana that referenced this pull request Aug 25, 2017
…d losing user context (elastic#13695)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
nreese added a commit that referenced this pull request Aug 25, 2017
…d losing user context (#13695) (#13710)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
nreese added a commit that referenced this pull request Aug 25, 2017
…d losing user context (#13695) (#13711)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
nreese added a commit that referenced this pull request Aug 25, 2017
…d losing user context (#13695) (#13712)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
@nreese nreese changed the title Keep current cursor or top line in view when resizing Console to avoid losing user context Keep top line in view when resizing Console to avoid losing user context Aug 28, 2017
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 20, 2017
…d losing user context (elastic#13695)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Dec 1, 2017
…d losing user context (elastic#13695)

* keep current cursor or top line in view when resizing to avoid losing user context

* rename variable to more descriptive name

* move state from editor to smart_resize
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

moving divider pane in console loses cursor focus
3 participants