Code folding UI & code style tweaks #10859
Conversation
There was a problem hiding this comment.
editor._codeMirror can never be null for any Editor, so I removed this check (the editor arg itself could be null -- e.g. in rare case where the "documentRefreshed" listener fires on a Document with no _masterEditor -- so I left that check above in place.
|
A few other things we should probably do before this story closes down:
I'm happy to help with those three. Lastly, @abose are you already tracking a task for creating unit tests to go with this feature? If not, should we create a bug or Trello card to track that work? |
|
@peterflynn Happy with these changes. I'll update PR #10850 later this evening and you can merge this. |
|
@peterflynn just to let you know I updated PR #10850. Merge your changes whenever you're ready. |
|
@peterflynn You got point 2 wrong for dark themes. Collapsed ones are brighter (= more outstanding) than expanded ones. |
|
@marcelgerber That's by design -- collapsed should be more prominent. So in the default light theme collapsed = darker, while in the dark theme collapsed = lighter. Third-party themes can override these styles though. I'll reword the bullet at top for clarity. |
|
Oh sorry, I got those wrong. What I meant was: |
|
@marcelgerber Hmm, that doesn't repro for me:
Do you have a third-party theme installed that might be interfering with the styles, or anything? |
peterflynn
force-pushed
the
pflynn/codefolding-tweaks
branch
from
6bc9cd0 to
82afac3
Compare
|
@thehogfather Fyi, I pushed a few more updates to this PR -- see updated description at top for details and a link to the correct diff view. It should now enable/disable dynamically much more cleanly, keeping the menus in proper order better and working better with splitview (and hopefully with slightly simpler code). I've rebased on top of your PR now, so until that's merged, this PR here will show the diff from both combined. |
|
|
|
@peterflynn Yeah you're right, it's an issue with my third-party theme (https://github.com/Brackets-Themes/Lion). |
- Fainter, smaller gutter triangle icons - Gutter triangle icons get less faint on mouseover (or when collapsed) - Vertically align gutter triangle icons with text better - Change inline collapsed placeholder to more readable arrow icon - Better colors in dark theme - Less horizontal gap between line numbers and triangle icons (without reducing gap between line numbers and text, if folding gutter disabled) Usability: rename "fadeFoldButtons" pref to clearer "hideUntilMouseover" And tweak code style issues: - Remove most of ignored, unneeded package.json - Preserve original CodeMirror copyright on CM-derived files - Rename a few things for clarity - Remove some unneeded code - Tweak formatting & spelling in comments
checking whether each bit has already initialized, atomically track whether init is done yet or not. Init/deinit all extant editors at once instead of doing each editor as it is activated, since splitview means editors may be already visible but not yet active. Fix listener leaks from repeated inits. And keep menu items in a more predictable order: - Add menu items at startup before user extensions load (at htmlReady instead of appReady), so core menu items like these don't move around as user extension load order changes - Place QuickView menu item more explicitly, so it doesn't move unpredictably relative to Code Folding menu items (within the extensions/default folder, load order is arbitrary) - Properly clean up menu divider, which used to get duplicated on each init pass
Remove unused `#code-folding-settings-dialog` CSS rule Fix some comment whitespace
peterflynn
force-pushed
the
pflynn/codefolding-tweaks
branch
from
82afac3 to
43a3ed1
Compare
|
@abose What OS are you on? Does it show the other arrow icon (that the code on master uses) properly? |
|
Windows 8.1. yes, it shows the arrow icon as in master properly. |
|
@peterflynn I was only fixing the unit test failures caused by the code folding extension in master. I think it's best to add an issue to add unit test cases here as this is one of the core features now. |
|
Now that PR #10850 has been merged, can this be merged? |
|
@abose Fixing the "code folding state doesn't get saved" issue (#10850 (comment)) is probably also something @peterflynn wants to fix in this PR (it's easy, even). |
|
@peterflynn pull #10885 has the fix for the issue (#10850 (comment)), being a simple it can be done as part of this PR itself - or merge that pr after merging this . |
|
Let me just fix the Unicode issue @abose pointed out -- then I think this is good to merge. |
|
Ok, I spent a while playing with this and decided to switch from arrows to a simple "...":
I actually like this a bit better than the wide arrows this PR had previously:
And it's definitely way more legible than the original narrow arrows:
|
Windows may not have glyphs for it and simpler Unicode arrows don't look good. Use simple "..." text instead (vertically aligned so it's more like an icon than text).
Code folding UI & code style tweaks
|
Thanks @peterflynn for the quick fix. |
We should land PR #10850 first, and then the diff for this will be simplified.(done)For now, this link previews what the diff will be after 10850 lands: thehogfather/brackets@issue10846...adobe:pflynn/codefolding-tweaksImprove visual appearance of code folding:
Usability: rename
"fadeFoldButtons"pref to clearer"hideUntilMouseover"And tweak code style issues:
Update - adds a few more fixes/cleanups:
More rigorous enable/disable flow for code folding: instead of piecemeal checking whether each bit has already initialized, atomically track whether init is done yet or not. Init/deinit all extant editors at once instead of doing each editor as it is activated, since splitview means editors may be already visible but not yet active. Fix listener leaks from repeated inits.
And keep menu items in a more predictable order: