Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow stacking the picker preview vertically #7783

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+52 −20
Open

allow stacking the picker preview vertically #7783

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 52 additions & 20 deletions helix-term/src/ui/picker.rs
View file
Original file line number Diff line number Diff line change
@@ -56,7 +56,10 @@ use self::handlers::{DynamicQueryChange, DynamicQueryHandler, PreviewHighlightHa

pub const ID: &str = "picker";

pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72;
pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 40;
pub const MIN_AREA_HEIGHT_FOR_PREVIEW: u16 = 9;
Copy link
Member

Choose a reason for hiding this comment

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

9 seems a little low, I don't think we should not below 15 here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one of the main goals here is to have it able to show the preview in an 80x24 terminal. would it be better to drop the min width down to 36? (it's worth noting that the interpretation of these constants is different in the pr than previously - the existing code tests against the full width/height, but in this pr it is testing against the width that would be used by just the preview)

Copy link
Member

Choose a reason for hiding this comment

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

ah I see I don't have time to test this out at the moment and din\t look at the new code in detail zet so I thaught it had the same meaning as before. I think the old bound was fine so if the definition just changed then its probably ok, I will take a look at that one I have more time.

I just meanr rhar 9 rows for picker and preview would have been a little low (that would have been like 1 or two matches shown at most)

pub const MAX_AREA_WIDTH_FOR_PREVIEW: u16 = 80;
pub const MAX_AREA_HEIGHT_FOR_PREVIEW: u16 = 24;
/// Biggest file size to preview in bytes
pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024;

@@ -934,28 +937,57 @@ impl<T: 'static + Send + Sync, D: 'static + Send + Sync> Picker<T, D> {

impl<I: 'static + Send + Sync, D: 'static + Send + Sync> Component for Picker<I, D> {
fn render(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) {
// +---------+ +---------+
// |prompt | |preview |
// +---------+ | |
// |picker | | |
// | | | |
// +---------+ +---------+

let render_preview =
self.show_preview && self.file_fn.is_some() && area.width > MIN_AREA_WIDTH_FOR_PREVIEW;

let picker_width = if render_preview {
area.width / 2
let (show_preview, vertical) = if area.width / 2 >= MIN_AREA_WIDTH_FOR_PREVIEW
&& area.height / 2 >= MIN_AREA_HEIGHT_FOR_PREVIEW
{
if area.width < area.height {
(self.show_preview, true)
} else {
(self.show_preview, false)
}
} else if area.width >= MIN_AREA_WIDTH_FOR_PREVIEW
&& area.height / 2 >= MIN_AREA_HEIGHT_FOR_PREVIEW
{
(self.show_preview, true)
} else if area.width / 2 >= MIN_AREA_WIDTH_FOR_PREVIEW
&& area.height >= MIN_AREA_HEIGHT_FOR_PREVIEW
{
(self.show_preview, false)
} else {
area.width
(false, false)
};

let picker_area = area.with_width(picker_width);
self.render_picker(picker_area, surface, cx);

if render_preview {
let preview_area = area.clip_left(picker_width);
self.render_preview(preview_area, surface, cx);
if show_preview && self.file_fn.is_some() {
if vertical {
// +---------------------+
// |prompt |
// +---------------------+
// |picker |
// | |
// +---------------------+
// |preview |
// | |
// +---------------------+
let preview_height = (area.height / 2).min(MAX_AREA_HEIGHT_FOR_PREVIEW);
let picker_height = area.height - preview_height;

self.render_picker(area.with_height(picker_height), surface, cx);
self.render_preview(area.clip_top(picker_height), surface, cx);
} else {
// +---------+ +---------+
// |prompt | |preview |
// +---------+ | |
// |picker | | |
// | | | |
// +---------+ +---------+
let preview_width = (area.width / 2).min(MAX_AREA_WIDTH_FOR_PREVIEW);
let picker_width = area.width - preview_width;

self.render_picker(area.with_width(picker_width), surface, cx);
self.render_preview(area.clip_left(picker_width), surface, cx);
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}

} else {
self.render_picker(area, surface, cx);
}
}