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.

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

NewFilePicker().Height seems to be ignored and filepicker overtakes containing form #470

Open
hxlnt opened this issue Nov 18, 2024 · 2 comments
Labels
bug Something isn't working filepicker

Comments

@hxlnt
Copy link

hxlnt commented Nov 18, 2024

Describe the bug

  1. When setting NewFilePicker().Picking(false).Height(*someInt*) or NewFilePicker().Picking(false) (no height set) or NewFilePicker().Height(*someInt*), the file picker expands (when activated) to the height of its container group or form. It also opens in a "new screen" rather than existing in place in its containing form. See screenshot below.
  2. When setting NewFilePicker().Picking(true).Height(*someInt*) or NewFilePicker().Picking(true)` (no height set), the returned height is always the full terminal height. It also opens in a "new screen" rather than existing in place in its containing form. See screenshot below.

To Reproduce
Steps to reproduce the behavior:

  1. When Picking is false:
huh.NewForm(
	huh.NewGroup(
		huh.NewNote().
			Description("life add electronics"),
		huh.NewFilePicker().
			Title("Datasheet file  ").
			Height(5).
			CurrentDirectory("../collections/electronics/files").
			ShowPermissions(false).
			DirAllowed(false).
			FileAllowed(true).
			Value(&tmpDatasheetPath),
		huh.NewInput().
			Value(&tmpFilename).
			Title("Filename"),
		huh.NewText().
			Value(&tmpNote).
			Placeholder("Best day ever.").
			Title("Optional notes").
			CharLimit(0).
			Lines(3),
	),
).Run()
  1. When Picking is true:
    (Code is same as above but with the addition of Picking(true). after huh.NewFilePicker()..)

Screenshots

  1. When Picking is false:
    Screenshot 2024-11-18 at 8 20 36 AM Screenshot 2024-11-18 at 8 20 48 AM
    When activated, the filepicker is displayed in a new screen that is the same height as its containing form or group.
  2. When Picking is true:
    Screenshot 2024-11-18 at 8 18 50 AM
    When the form is displayed, the filepicker automatically overtakes the form and displays in a new screen that is the entire height of the terminal.

Expected behavior

  1. Firstly, I didn't expect the filepicker to open in its own "window"... I thought it would operate in place like a Select. This would be my preference.
  2. Secondly, I would expect the Height() to impact the rendering height of the filepicker (again, in place on the form if possible).

Desktop (please complete the following information):

  • OS: Mac OS

Additional context
I feel like I've seen the filepicker work as expected in this version of huh, so I wonder if there's some additional aspect that's causing this behavior such its containing group or form.

@hxlnt
Copy link
Author

hxlnt commented Nov 18, 2024

EDIT: Sorry, just noticed this might be related to 9eaf750. The bug report above is for the latest source code (not the latest binary), which includes the commit above. I realize now that this feature might still be WIP, but I'll leave the bug report here in case it's helpful.

(This might also be why I thought it was working differently a few days ago.)

@caarlos0 caarlos0 added bug Something isn't working filepicker labels Nov 26, 2024
@bashbunni
Copy link
Member

bashbunni commented Nov 27, 2024

Just an update on this - the issue here is that when the filepicker's "open" command is used, the field is zoomed which ignores manual height settings. Here's a patch that fixes your issue while maintaining compatibility for auto-sizing if there is no height set (key for Gum).

Only thing is that for some reason the number of items shown is off by one from the provided height... Can debug that one more. Wasn't an issue with custom heights in the latest release of Gum 🤔

To explain the change in english:
Add adjust() helper function needed in both Height and WithHeight functions. The key diff btwn those functions now is WithHeight is used by group, therefore its value is not considered a manual height setting. By contrast, Height set to a non-zero value will flag the picker to disable AutoHeight. Also added a check to see if there's a manual height set, don't Zoom because it will ignore the manual height and instead size to the window.

maintain-height.patch

diff --git a/field_filepicker.go b/field_filepicker.go
index 36a3385..2187951 100644
--- a/field_filepicker.go
+++ b/field_filepicker.go
@@ -145,9 +145,7 @@ func (f *FilePicker) AllowedTypes(types []string) *FilePicker {
 	return f
 }
 
-// Height sets the height of the file field. If the number of options
-// exceeds the height, the file field will become scrollable.
-func (f *FilePicker) Height(height int) *FilePicker {
+func (f FilePicker) adjust() int {
 	adjust := 0
 	if f.title != "" {
 		adjust++
@@ -155,7 +153,19 @@ func (f *FilePicker) Height(height int) *FilePicker {
 	if f.description != "" {
 		adjust++
 	}
-	f.picker.Height = height - adjust
+	return adjust
+}
+
+// Height sets the height of the file field. If the number of options exceeds
+// the height, the file field will become scrollable.
+//
+// This function forces the file field to use the manual height set by the user
+// if one is set.
+func (f *FilePicker) Height(height int) *FilePicker {
+	if height != 0 {
+		f.picker.AutoHeight = false
+	}
+	f.picker.Height = height - f.adjust()
 	return f
 }
 
@@ -177,7 +187,8 @@ func (*FilePicker) Skip() bool {
 
 // Zoom returns whether the input should be zoomed.
 func (f *FilePicker) Zoom() bool {
-	return f.picking
+	// Don't zoom if user has set a custom height.
+	return f.picking && f.picker.AutoHeight
 }
 
 // Focus focuses the file field.
@@ -398,10 +409,10 @@ func (f *FilePicker) WithWidth(width int) Field {
 	return f
 }
 
-// WithHeight sets the height of the file field.
+// WithHeight sets the height of the file field. Used by group.
 func (f *FilePicker) WithHeight(height int) Field {
 	f.height = height
-	f.Height(height)
+	f.picker.Height = height - f.adjust()
 	f.picker, _ = f.picker.Update(nil)
 	return f
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working filepicker
Projects
None yet
Development

No branches or pull requests

3 participants