Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Allow folds to be toggled by clicking icons in the gutter #1441

Merged
merged 25 commits into from
Jan 20, 2014

Conversation

nathansobo
Copy link
Contributor

Left to do:

  • Rename fold class to folded to match foldable
  • Indicate that multi-line comment blocks are foldable
  • Update foldable indicators when the indentation of a line changes
  • Update foldable indicators when a line is commented / uncommented
  • One last look at the CSS. Can it be simpler?
  • Fix positioning of the end of line fold indicators
  • Don't mark a commented line as foldable just because the next line is indented
  • Fix apm "base" theme
  • Publish new bookmarks package and upgrade to it
  • Remove backward-compatible shims from syntax themes after merging

fold-indicators

I changed the layout of the gutter a bit to make the fold icon a real element rather than using pseudo elements. This is because I needed to detect if the icon specifically was clicked. I also opened up a bit more space in the gutter so things don't feel too squished.

@gutter.on 'mousedown', '.foldable .fold-icon', (e) =>
bufferRow = $(e.target).parent().data('bufferRow')
@editor.toggleFoldAtBufferRow(bufferRow)
return false
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need the return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What got into me? Amateur hour.

@kevinsawicki
Copy link
Contributor

Does this add any overhead to the rendering time of the gutter to lookup the foldability of the row?

@nathansobo
Copy link
Contributor Author

It adds some, but not much. Here's a profile of scrolling through jQuery from top to bottom, which invokes the gutter update every time we page in a new group of lines. It's about 4.5% of the total time handling scroll events. I can look into caching it... what do you think?

screen shot 2014-01-17 at 3 52 43 pm

@benogle
Copy link
Contributor

benogle commented Jan 17, 2014

Looks rad.

How does this work with bookmarks?

@nathansobo
Copy link
Contributor Author

@benogle: Good question. I need to check that out.

@nathansobo
Copy link
Contributor Author

@benogle: Thanks for the heads up. Looks like bookmarks are totally hosed. Any ideas? We could...

  • Move the bookmark to the left of the numbers.
  • Hide the bookmark when the user hovers. But it needs to be clickable as well right?

@kevinsawicki
Copy link
Contributor

Refs #907

@benogle
Copy link
Contributor

benogle commented Jan 17, 2014

Bookmarks dont need to be clickable. Maybe left of the numbers provided there isnt too much crowding? Or maybe some other indicator like a colored background on the line number?

i say try them on the left first.

@nathansobo
Copy link
Contributor Author

@benogle If they don't need to be clickable I'd prefer for them to supersede the fold indicators until the gutter is hovered. Otherwise we need to make the gutter quite a bit wider either always or when a bookmark is enabled.

@benogle
Copy link
Contributor

benogle commented Jan 17, 2014

Ok. If it is weird, we can change it.

@kevinsawicki
Copy link
Contributor

In the gif I don't see the icon on 601 but multi-line doc blocks are foldable I thought.

@nathansobo
Copy link
Contributor Author

@kevinsawicki Yeah good I didn't add the indicators for docs comment blocks.
@benogle: What do you think of this bookmarks interaction?

hovers

@benogle
Copy link
Contributor

benogle commented Jan 18, 2014

I like this. I like how they dont show up at all til you hover, and the blue arrows when bookmarked.

@nathansobo
Copy link
Contributor Author

I now realize this feature requires foldability to be something we track persistently, whereas before we were determining it at a specific point in time. Going to need to bake it deeper into the cascade, probably associated with TokenizedBuffer so we can update it when text or tokenization changes.

This was referenced Sep 16, 2021
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.

None yet

3 participants