Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Add code folding to the editor - #2468

Merged
jasonLaster merged 5 commits into
firefox-devtools:masterfrom
wldcordeiro:code-folding
Apr 7, 2017
Merged

Add code folding to the editor#2468
jasonLaster merged 5 commits into
firefox-devtools:masterfrom
wldcordeiro:code-folding

Conversation

@wldcordeiro

Copy link
Copy Markdown
Contributor

Associated Issue: #2465

Summary of Changes

  • Adds code folding to the editor

Test Plan

  • Open a document
  • Click the folding indicators in the gutter
  • Note that clicking the indicator doesn't add/remove breakpoints

Screenshots/Videos

code-fold

require("codemirror/addon/search/searchcursor");
require("codemirror/addon/fold/foldcode");
require("codemirror/addon/fold/brace-fold");
require("codemirror/addon/fold/indent-fold");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can add xml-fold as well that may be a nice touch for HTML files (and maybe JSX? haven't tested this though), can also add comment-fold too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like a good set for now

@codecov

codecov Bot commented Mar 27, 2017

Copy link
Copy Markdown

Codecov Report

Merging #2468 into master will increase coverage by 0.01%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2468      +/-   ##
==========================================
+ Coverage    57.7%   57.72%   +0.01%     
==========================================
  Files          51       51              
  Lines        1986     1994       +8     
  Branches      398      399       +1     
==========================================
+ Hits         1146     1151       +5     
- Misses        840      843       +3
Impacted Files Coverage Δ
src/utils/editor/source-editor.js 31.37% <100%> (+5.84%) ⬆️
src/utils/editor/index.js 15.38% <25%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6365d3d...eacf861. Read the comment docs.

@jasonLaster jasonLaster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is pretty cool. And it's another reason why using codemirror is so great

I'll add some thoughts on the general UX/UI below

Comment thread src/utils/editor/index.js Outdated
function createEditor() {
return new SourceEditor({
mode: "javascript",
foldGutter: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feel free to feature flag this while we work on the UX/UI

@jasonLaster

Copy link
Copy Markdown
Contributor

There are a couple style regressions:

  • the breakpoint marker lost the line number
  • the fold marker needs padding and a lighter color
  • the code fold gutter column seems to wide

Here's atom's fold gutter. Some points I like about it are:

  1. fold markers are hidden except when hovering over the gutter
  2. active markers are orange and always visible
  3. the fold inline badge is really nice

I'd like to iterate once we address some of the regressions

@wldcordeiro

Copy link
Copy Markdown
Contributor Author

@jasonLaster I'm pushing some changes that fix the regressions now.

Some notes on the enhancements though...

fold markers are hidden except when hovering over the gutter

I'm not sure this is easily doable, the gutter in CodeMirror is weird in that the background is one element .CodeMirror-gutters and the actual content of the gutter is within a completely different adjacent element (maybe a very specific selector heh?)

active markers are orange and always visible

I like the idea of some kind of highlight for the active marker. 👍

the fold inline badge is really nice

I do like their badge, it is pretty nice looking compared to the one we inherited from codemirror-mozilla.css

@jasonLaster jasonLaster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So cool.

Small nit, then lets do it

position: absolute;
top: 0px;
right: -4px;
right: -16px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we feature flag this CSS...

.has-code-folding.editor.new-breakpoint {
  right: -16px;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. 👍

/* move the breakpoint below the linenumber */
.new-breakpoint .CodeMirror-gutter-elt:last-child {
/* move the breakpoint below the other gutter elements */
.new-breakpoint .CodeMirror-gutter-elt:nth-child(2) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oof

wldcordeiro and others added 2 commits April 6, 2017 20:25
…ng disabled.

Signed-off-by: Wellington Cordeiro <wellington@wellingtoncordeiro.com>
@jasonLaster
jasonLaster merged commit 60e2ce0 into firefox-devtools:master Apr 7, 2017
@jasonLaster

jasonLaster commented Apr 7, 2017

Copy link
Copy Markdown
Contributor

Thanks @wldcordeiro! this is really cool. Lets write up some smaller issues

@wldcordeiro
wldcordeiro deleted the code-folding branch April 7, 2017 03:07
@jasonLaster jasonLaster mentioned this pull request Apr 7, 2017
7 tasks
DanUgelow pushed a commit to DanUgelow/debugger.html that referenced this pull request May 4, 2017
* Add code folding to the editor

* Fix line number hiding under breakpoint marker regression
Extend breakpoint marker past code folding marker

* Feature flag code folding

* Feature flag some of the CSS to render the gutter properly with folding disabled.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants