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

Improve keyboard movement in project results pane #498

Merged
merged 13 commits into from Oct 1, 2015

Conversation

Projects
None yet
2 participants
@benogle
Contributor

benogle commented Aug 14, 2015

This turned into kind of a rabbit hole. My intent was to fix move-to-bottom and add move-to-top.

Changes:

  • Fixes core:move-to-bottom to actually work
  • Adds core:move-to-top support
  • Adds core:page-down support
  • Adds core:page-up support
  • Allow selecting paths even when they are not collapsed
  • Fixes movement when files have been updated. Updating a file so it no longer has matches for the search string removes old results from the ResultsView by hiding the ResultView element for the path. Previously, it would attempt to select these invisible elements, which would make the selection 'stick' in the removed element location when arrowing through the results.

benogle added a commit that referenced this pull request Oct 1, 2015

Merge pull request #498 from atom/bo-results-movement
Improve keyboard movement in project results pane

@benogle benogle merged commit 756a85d into master Oct 1, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@benogle benogle deleted the bo-results-movement branch Oct 1, 2015

@as-cii

This comment has been minimized.

Show comment
Hide comment
@as-cii

as-cii Oct 13, 2015

Member

@benogle: I spotted this line of code on master which seemed a bit suspicious and found out it was added in this commit. I think the result of the @lastRenderedResultIndex - @lastRenderedResultIndex expression will always be 0, unless I am missing something very obvious. In such case, please disregard this comment. 🙏

Otherwise, do you think there could be a typo or something here?

Member

as-cii commented on lib/project/results-view.coffee in fe63993 Oct 13, 2015

@benogle: I spotted this line of code on master which seemed a bit suspicious and found out it was added in this commit. I think the result of the @lastRenderedResultIndex - @lastRenderedResultIndex expression will always be 0, unless I am missing something very obvious. In such case, please disregard this comment. 🙏

Otherwise, do you think there could be a typo or something here?

This comment has been minimized.

Show comment
Hide comment
@benogle

benogle Oct 15, 2015

Contributor

Ugh, I think it's suppose to be the stop condition:

else if renderNext is @lastRenderedResultIndex - initialIndex

Which would break when the number of rows rendered is equal to the number of rows that were requested to be rendered. So right now, it looks like it is rendering to the end in all cases. Likely slow with large result sets.

Contributor

benogle replied Oct 15, 2015

Ugh, I think it's suppose to be the stop condition:

else if renderNext is @lastRenderedResultIndex - initialIndex

Which would break when the number of rows rendered is equal to the number of rows that were requested to be rendered. So right now, it looks like it is rendering to the end in all cases. Likely slow with large result sets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment