feat(gutter): added keyboard handling for experimental feature custom widgets #5796
Conversation
nlujjawal
requested review from
akoreman and
nightwing
and removed request for
akoreman
nlujjawal
force-pushed
the
gutter_keyboard
branch
from
1d8bcbc to
7de95fe
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5796 +/- ##
==========================================
+ Coverage 87.32% 87.34% +0.02%
==========================================
Files 605 605
Lines 44527 44620 +93
Branches 7287 7301 +14
==========================================
+ Hits 38881 38975 +94
+ Misses 5646 5645 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -205,6 +206,7 @@ class GutterKeyboardHandler { | |||
| switch (this.activeLane){ | |||
| case "fold": | |||
| this.$blurFoldWidget(this.activeRowIndex); | |||
There was a problem hiding this comment.
Does $blurFoldWidget here expect there always to be a foldwidget at the provided row or will it handle the possibility of no fold widget being there okay?
There was a problem hiding this comment.
It handles the possibility of no fold widget here.
src/keyboard/gutter_handler.js
Outdated
| @@ -265,6 +279,28 @@ class GutterKeyboardHandler { | |||
| return null; | |||
| } | |||
|
|
|||
| // Given an index, find the nearest index with a customwidget | |||
| $findNearestCustomWidget(index) { | |||
There was a problem hiding this comment.
bit of a nitpick given that these are all private methods, but given that fold widgets and custom widgets are mutually exclusive (a row can only either have a fold widget or a custom widget), would it make sense to just have a single method findNearestFoldLaneWidget (or something along those lines) to find whatever widget is closest in the fold lane (same for all other methods involving fold and custom widgets)
There was a problem hiding this comment.
Ack.
Merging nearestCustomWidget and nearestFoldWidget makes sense, for other methods it doesn't make much sense as it will increase the lines of code requried.
src/keyboard/gutter_handler.js
Outdated
| var i = 0; | ||
| while (index - i > 0 || index + i < this.lines.getLength() - 1){ | ||
| i++; | ||
| if (index - i >= 0 && this.editor.session.$customWidgets[this.$rowIndexToRow(index - i)]) |
There was a problem hiding this comment.
Why aren't we using isCustomWidgetVisible here?
There was a problem hiding this comment.
Yeah updated, both things can be same its equally same but for consistency updated that.
nlujjawal
force-pushed
the
gutter_keyboard
branch
from
32f7d0d to
35f42a3
Compare
Description of changes:
Added keyboard handling for experimental feature custom widgets which required changes for the code which handles when gutter is active and user presses enter, up, down, space etc.
Its a followup to this #5787 where in we added experimental ability of replacing the fold widgets with custom widgets.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.