Cursor improvements and placeholder text customization#10294
Cursor improvements and placeholder text customization#10294alokedesai wants to merge 2 commits into
Conversation
…tory - Add RichTextStylesExt trait with new_with_default_line_height for comment editors - Add IndentableBlockSpacing::new() and BlockSpacings::uniform() constructors - Add style_factory field to RichTextEditorConfig/RichTextEditorView - Consolidate style_factory with initial styles in comment_editor - Compact text block spacings, 3px cursor width, remove minimum_paragraph_height Co-Authored-By: Oz <oz-agent@warp.dev>
- Fix cursor clipping at position 0 by clamping bar cursor origin - Fix trailing newline cursor to use styles.cursor_width - Add PlaceholderVisibility enum (Enabled/Disabled/WhenBufferEmpty) - Add custom placeholder_text field to RichTextStyles - Use PlaceholderVisibility::Disabled for code editors - Use WhenBufferEmpty + 'Leave a comment' for comment editors - Update placeholder.rs, paragraph.rs, empty.rs for new visibility logic Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR updates rich-text placeholder configuration, customizes comment placeholder text, and adjusts cursor rendering/clamping behavior.
Concerns
- Removing
CodeEditorView::active_cursor_positioncauses code editors to stop reporting cursor coordinates to the platform, which regresses IME candidate positioning. WhenBufferEmptycurrently treats multiple blank blocks as empty, so the custom placeholder can remain visible after a user has inserted blank lines.- This is UI-impacting, but the PR description has no screenshots or video. For faster review, please upload screenshots or a video of the feature working end to end.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
| } | ||
|
|
||
| fn active_cursor_position(&self, ctx: &ViewContext<Self>) -> Option<CursorInfo> { |
There was a problem hiding this comment.
View::active_cursor_position fall back to None for code editors, so IME/composition candidate windows lose the cursor anchor; keep reporting the saved cursor position here or replace it with an equivalent implementation.
| PlaceholderVisibility::Enabled => self.show_always || contains_cursor, | ||
| PlaceholderVisibility::WhenBufferEmpty => { | ||
| // Show only when every block in the buffer is empty. | ||
| let all_empty = model.content().block_items().all(BlockItem::is_empty); |
There was a problem hiding this comment.
all(BlockItem::is_empty) still returns true for buffers with multiple blank paragraphs/trailing empty blocks, so WhenBufferEmpty can keep showing the placeholder after the user adds blank lines; check for a single user-editable empty block instead.
| } | ||
|
|
||
| // If the character is opening autocomplete symbol, we want to autocomplete it with a closing symbol. | ||
| // If the character is opening autcomplete symbol, we want to autocomplete it with a closing symbol. |
There was a problem hiding this comment.
💡 [SUGGESTION] Fix the typo introduced in this comment.
| // If the character is opening autcomplete symbol, we want to autocomplete it with a closing symbol. | |
| // If the character is opening autocomplete symbol, we want to autocomplete it with a closing symbol. |
1 participant
Description
Linked Issue
ready-to-specorready-to-implement.Screenshots / Videos
Testing
Agent Mode