editor: Add minimap #26893
editor: Add minimap #26893
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @esimkowitz on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
cla-bot
bot
added
the
cla-signed
|
The cla-bot has been summoned, and re-checked this pull request! |
need to work on how it displays and make sure it doesn't cover text
There was a problem hiding this comment.
Then you a lot for coming with the proposal, quite exciting to see a prototype that looks nice.
Have left a few notes in the PR and below, but overall the approach seems working to me, I've run the profiler along the release build of this branch and saw no issue with anything editor-related.
Can you elaborate on the performance issues you observe?
Overall, the last big missing piece of functionality is mouse click handlers, what do you think?
If so, let's concentrate on that and the rest "functional" items first, so we can polish the design a bit later, using the help of the designers?
There's a bug causing the minimap to bounce off the edge:
jump.mov
Seems related, as we also have a centered layout feature, which makes the minimap hover over the code:
While noted in the PR description, I'd like to emphasize for the bright themes, how now would be to have a background/border to visually separate minimap from the rest of the text.
I like Sublime Text 2's approach, where the viewport is surrounded with the border (notice the thumb placement too)
|
I think the performance implication with scrolling that I was noticing was actually just a standard degradation of the debug environment vs the release environment - I don't notice it when I run a release build so no concern there. I do notice some jumping, though, I think it may be related to the issue you noted where the minimap detaches. I'll take a look at this and the other notes you've made, thank you for such a detailed review! |
Thinking some more on this, what if we swap every character for a block that is the full width of the em? I also like the idea of the bracketed highlighting, will look into this. I think this can work well |
|
Thankfully the bouncing issue was simple to fix, I was using |
|
Okay big update, I have the display logic working I think, including the slider thumb. It isn't interactive yet, but if folks could try it out and let me know if they notice any spookies, I'd really appreciate it! |
|
I'm noticing that even though I'm using the scrollbar bounds as the basis for the minimap bounds, sometimes the height is off. I'll dig into this some more with the debugger |
There was a problem hiding this comment.
Thank you again @esimkowitz for validating the originally posted approach description: that have indeed worked and hopefully was not that hard and fun to work on.
And thank you again @MrSubidubi for those gnarly scroll-related issues tackled — ironically, the hardest part of the PR.
Ready to merge it when the conflicts & CI are dealt with.
|
Currently, the source code art evan's repo fails to compile
And before that I had a crash the moment I opened a file to edit it. |
|
See #30049 as well as #30138 . Once these are in (or something similar is agreed upon), we should be ready to go. Please note that both PRs are currently blocking, as seen in the comment above (and the description below). @mocenigo This is currently expected and happens due to b574129 and #30049 not being in yet, which are needed because of #26893 (comment). Once we have these two issues sorted out, we are good to go again. |
Ok. When do you expect to have the minimap merged into zed’s main? |
… horizontal content padding (#30138) This PR changes the way a horizontal margin is added in editors. It removes the possibility to set a custom `horizontal_padding` for an editor and utilizes the default `gutter_dimension` instead. This change is made to ensure that no issues with soft-wrapping occurs for any editor that has a `horizontal_margin` set (see #26893 for more context on the implications here`. Furthermore, it ensures that the text actually renders properly when scrolling horizontally and is not cut-off. ### Horizontal padding: | `main` | This PR | | --- | --- | |  |  | ### Editor horizontally scrolled: | `main` | This PR | | --- | --- | |  |  | Notice the difference at the horizontal borders. The margin added for the `edit_file_tool` was 4 pixels. The `descent`, whilst not exactly, is roughly the same here and also scales with the font size nicely. Furthermore, it seems that the `gutter_dimensions.margin` should be present anyway, given the following comment https://github.com/zed-industries/zed/blob/0b00256f5884fd17b4f834730b31f365613f3683/crates/editor/src/element.rs#L6887-L6889 so ensuring this property is actually set and not 0 seems to be reasonable given the circumstances. Please note though that this will apply to all editors in the app. Again, this seems like it should be the case anyway, just wanted to mention this again. Should the fix like this not be wanted, I can change this here so that the `horizontal_margin` is better accounted for when soft-wrapping in an editor. Feel free to let me know in this case. Release Notes: - N/A
We are good to go once #30049 is reviewed and approved. |
Noticed this whilst working on #26893 This PR prevents that single line and auto height editors have a conflict addon attached (and are observed for any excerpt changes). From how I understand it, it does not really make sense to register the conflict addon for single line or auto height editors. These editors will never show a conflict nor will they be used to resolve one. Furthermore, neither of these ever have a project attached upon creation: https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/editor/src/editor.rs#L1385 https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/editor/src/editor.rs#L1403 https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/editor/src/editor.rs#L1415 so their buffers will never be added here: https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/git_ui/src/conflict_view.rs#L116-L120 Thus, we could potentially even extend the check with an additional `|| editor.project.is_none()`. Yet, as I am not entirely sure how all of this exactly works, I left this out for now, but I can definitely add this if wanted. Release Notes: - N/A
|
Yay! Lots of love to you guys! |
…maller than viewport (#30189) As discussed and explained in #26893 (comment) This PR fixes an issue where we would have zero-divisions during scrollbar layouting for small files. This happened due to the fact that for small files, https://github.com/zed-industries/zed/blob/9c1b2afa492755a1e8cb0a77f929845941c97cdc/crates/editor/src/element.rs#L8562-L8563 would be `NaN`, since `(total_text_units - text_units_per_page).max(0.)` would return `0.`, which we would divide by. However, this was neccessary to be in place, as this prevented the scroll thumb from being rendered for small files: Due to this being `NaN`, the thumb origin would be `Pixels(NaN)`, which prevented the rendering of the scrollbar thumb. This PR fixes this behavior by accounting for this scenario and changing the thumb bounds to be an `Option<Bounds<Pixels>>` instead. This furthermore has the advantage that we have to compute the thumb only once and storing it in the layout, which was previously not possible. Most notably, this enables scrollbar markers to show for smaller files: https://github.com/user-attachments/assets/9fa5d240-8795-4fae-9933-aed144df4f5e Currently, no markers are shown due to the fact that `Pixels(NaN)` is set as the origin point. Also, I changed that the cursor style will only be changed on the scrollbar hitbox when we will actually show a thumb. This way, for small files (where viewport > content size) the cursor will not change when a user hovers with their mouse over the scrollbars hitbox. Theoretically, I could also include the change mentioned in #26893 (comment) here. Given the introduction of the minimap as well as #29316 and the cursor style taken care of here, removing the guard would not change anything and creates the possibility to soon introduce scrollbars for auto height editors. Please let me know whether we want to have this in this PR or whether I shall create a seperate one. Release Notes: - Enabled scrollbar marker rendering for small files.
|
I have notice that you forget to implement the |
| // The width of the minimap in pixels. | ||
| "width": 100, | ||
| // The font size of the minimap in pixels. | ||
| "font_size": 2 |
There was a problem hiding this comment.
no code to use these config
Closes #30250 Based on #26893 (comment) Release Notes: - N/A
Follow-up of #26893 Release Notes: - N/A
… horizontal content padding (zed-industries#30138) This PR changes the way a horizontal margin is added in editors. It removes the possibility to set a custom `horizontal_padding` for an editor and utilizes the default `gutter_dimension` instead. This change is made to ensure that no issues with soft-wrapping occurs for any editor that has a `horizontal_margin` set (see zed-industries#26893 for more context on the implications here`. Furthermore, it ensures that the text actually renders properly when scrolling horizontally and is not cut-off. ### Horizontal padding: | `main` | This PR | | --- | --- | |  |  | ### Editor horizontally scrolled: | `main` | This PR | | --- | --- | |  |  | Notice the difference at the horizontal borders. The margin added for the `edit_file_tool` was 4 pixels. The `descent`, whilst not exactly, is roughly the same here and also scales with the font size nicely. Furthermore, it seems that the `gutter_dimensions.margin` should be present anyway, given the following comment https://github.com/zed-industries/zed/blob/0b00256f5884fd17b4f834730b31f365613f3683/crates/editor/src/element.rs#L6887-L6889 so ensuring this property is actually set and not 0 seems to be reasonable given the circumstances. Please note though that this will apply to all editors in the app. Again, this seems like it should be the case anyway, just wanted to mention this again. Should the fix like this not be wanted, I can change this here so that the `horizontal_margin` is better accounted for when soft-wrapping in an editor. Feel free to let me know in this case. Release Notes: - N/A
…tries#30049) Noticed this whilst working on zed-industries#26893 This PR prevents that single line and auto height editors have a conflict addon attached (and are observed for any excerpt changes). From how I understand it, it does not really make sense to register the conflict addon for single line or auto height editors. These editors will never show a conflict nor will they be used to resolve one. Furthermore, neither of these ever have a project attached upon creation: https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/editor/src/editor.rs#L1385 https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/editor/src/editor.rs#L1403 https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/editor/src/editor.rs#L1415 so their buffers will never be added here: https://github.com/zed-industries/zed/blob/00c5f57575b5de69a5007ba724b4a8fb10db91ac/crates/git_ui/src/conflict_view.rs#L116-L120 Thus, we could potentially even extend the check with an additional `|| editor.project.is_none()`. Yet, as I am not entirely sure how all of this exactly works, I left this out for now, but I can definitely add this if wanted. Release Notes: - N/A
## Overview This PR adds the minimap feature to the Zed editor, closely following the [design from Visual Studio Code](https://code.visualstudio.com/docs/getstarted/userinterface#_minimap). When configured, a second instance of the editor will appear to the left of the scrollbar. This instance is not interactive and it has a slimmed down set of annotations, but it is otherwise just a zoomed-out version of the main editor instance. A thumb shows the line boundaries of the main viewport, as well as the progress through the document. Clicking on a section of code in the minimap will jump the editor to that code. Dragging the thumb will act like the scrollbar, moving sequentially through the document.  ## New settings This adds a `minimap` section to the editor settings with the following keys: ### `show` When to show the minimap in the editor. This setting can take three values: 1. Show the minimap if the editor's scrollbar is visible: `"auto"` 2. Always show the minimap: `"always"` 3. Never show the minimap: `"never"` (default) ### `thumb` When to show the minimap thumb. This setting can take two values: 1. Show the minimap thumb if the mouse is over the minimap: `"hover"` 2. Always show the minimap thumb: `"always"` (default) ### `width` The width of the minimap in pixels. Default: `100` ### `font_size` The font size of the minimap in pixels. Default: `2` ## Providing feedback In order to keep the PR focused on development updates, please use the discussion thread for feature suggestions and usability feedback: zed-industries#26894 ## Features left to add - [x] fix scrolling performance - [x] user settings for enable/disable, width, text size, etc. - [x] show overview of visible lines in minimap - [x] clicking on minimap should navigate to the corresponding section of code - ~[ ] more prominent highlighting in the minimap editor~ - ~[ ] override scrollbar auto setting to always when minimap is set to always show~ Release Notes: - Added minimap for high-level overview and quick navigation of editor contents. --------- Co-authored-by: MrSubidubi <[email protected]> Co-authored-by: Kirill Bulatov <[email protected]>
…maller than viewport (zed-industries#30189) As discussed and explained in zed-industries#26893 (comment) This PR fixes an issue where we would have zero-divisions during scrollbar layouting for small files. This happened due to the fact that for small files, https://github.com/zed-industries/zed/blob/9c1b2afa492755a1e8cb0a77f929845941c97cdc/crates/editor/src/element.rs#L8562-L8563 would be `NaN`, since `(total_text_units - text_units_per_page).max(0.)` would return `0.`, which we would divide by. However, this was neccessary to be in place, as this prevented the scroll thumb from being rendered for small files: Due to this being `NaN`, the thumb origin would be `Pixels(NaN)`, which prevented the rendering of the scrollbar thumb. This PR fixes this behavior by accounting for this scenario and changing the thumb bounds to be an `Option<Bounds<Pixels>>` instead. This furthermore has the advantage that we have to compute the thumb only once and storing it in the layout, which was previously not possible. Most notably, this enables scrollbar markers to show for smaller files: https://github.com/user-attachments/assets/9fa5d240-8795-4fae-9933-aed144df4f5e Currently, no markers are shown due to the fact that `Pixels(NaN)` is set as the origin point. Also, I changed that the cursor style will only be changed on the scrollbar hitbox when we will actually show a thumb. This way, for small files (where viewport > content size) the cursor will not change when a user hovers with their mouse over the scrollbars hitbox. Theoretically, I could also include the change mentioned in zed-industries#26893 (comment) here. Given the introduction of the minimap as well as zed-industries#29316 and the cursor style taken care of here, removing the guard would not change anything and creates the possibility to soon introduce scrollbars for auto height editors. Please let me know whether we want to have this in this PR or whether I shall create a seperate one. Release Notes: - Enabled scrollbar marker rendering for small files.
Closes zed-industries#30250 Based on zed-industries#26893 (comment) Release Notes: - N/A
Follow-up of zed-industries#26893 Release Notes: - N/A
… horizontal content padding (#30138) This PR changes the way a horizontal margin is added in editors. It removes the possibility to set a custom `horizontal_padding` for an editor and utilizes the default `gutter_dimension` instead. This change is made to ensure that no issues with soft-wrapping occurs for any editor that has a `horizontal_margin` set (see #26893 for more context on the implications here`. Furthermore, it ensures that the text actually renders properly when scrolling horizontally and is not cut-off. ### Horizontal padding: | `main` | This PR | | --- | --- | |  |  | ### Editor horizontally scrolled: | `main` | This PR | | --- | --- | |  |  | Notice the difference at the horizontal borders. The margin added for the `edit_file_tool` was 4 pixels. The `descent`, whilst not exactly, is roughly the same here and also scales with the font size nicely. Furthermore, it seems that the `gutter_dimensions.margin` should be present anyway, given the following comment https://github.com/zed-industries/zed/blob/0b00256f5884fd17b4f834730b31f365613f3683/crates/editor/src/element.rs#L6887-L6889 so ensuring this property is actually set and not 0 seems to be reasonable given the circumstances. Please note though that this will apply to all editors in the app. Again, this seems like it should be the case anyway, just wanted to mention this again. Should the fix like this not be wanted, I can change this here so that the `horizontal_margin` is better accounted for when soft-wrapping in an editor. Feel free to let me know in this case. Release Notes: - N/A
… horizontal content padding (#30138) This PR changes the way a horizontal margin is added in editors. It removes the possibility to set a custom `horizontal_padding` for an editor and utilizes the default `gutter_dimension` instead. This change is made to ensure that no issues with soft-wrapping occurs for any editor that has a `horizontal_margin` set (see #26893 for more context on the implications here`. Furthermore, it ensures that the text actually renders properly when scrolling horizontally and is not cut-off. ### Horizontal padding: | `main` | This PR | | --- | --- | |  |  | ### Editor horizontally scrolled: | `main` | This PR | | --- | --- | |  |  | Notice the difference at the horizontal borders. The margin added for the `edit_file_tool` was 4 pixels. The `descent`, whilst not exactly, is roughly the same here and also scales with the font size nicely. Furthermore, it seems that the `gutter_dimensions.margin` should be present anyway, given the following comment https://github.com/zed-industries/zed/blob/0b00256f5884fd17b4f834730b31f365613f3683/crates/editor/src/element.rs#L6887-L6889 so ensuring this property is actually set and not 0 seems to be reasonable given the circumstances. Please note though that this will apply to all editors in the app. Again, this seems like it should be the case anyway, just wanted to mention this again. Should the fix like this not be wanted, I can change this here so that the `horizontal_margin` is better accounted for when soft-wrapping in an editor. Feel free to let me know in this case. Release Notes: - N/A
|
Thank you @MrSubidubi @SomeoneToIgnore @shenjackyuanjie for pushing this through, sorry for disappearing at the end, I started a new job and unfortunately couldn't dedicate as much time to this. I love how it turned out! |
Overview
This PR adds the minimap feature to the Zed editor, closely following the design from Visual Studio Code. When configured, a second instance of the editor will appear to the left of the scrollbar. This instance is not interactive and it has a slimmed down set of annotations, but it is otherwise just a zoomed-out version of the main editor instance. A thumb shows the line boundaries of the main viewport, as well as the progress through the document. Clicking on a section of code in the minimap will jump the editor to that code. Dragging the thumb will act like the scrollbar, moving sequentially through the document.
New settings
This adds a
minimapsection to the editor settings with the following keys:showWhen to show the minimap in the editor.
This setting can take three values:
"auto""always""never"(default)thumbWhen to show the minimap thumb.
This setting can take two values:
"hover""always"(default)widthThe width of the minimap in pixels.
Default:
100font_sizeThe font size of the minimap in pixels.
Default:
2Providing feedback
In order to keep the PR focused on development updates, please use the discussion thread for feature suggestions and usability feedback: #26894
Features left to add
[ ] more prominent highlighting in the minimap editor[ ] override scrollbar auto setting to always when minimap is set to always showRelease Notes: