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

feat: add tests to verify initial rendering #25

Merged
merged 4 commits into from
Jan 23, 2024
Merged

Conversation

roele
Copy link
Collaborator

@roele roele commented Jan 23, 2024

@jdx Not sure how useful these could be as they only check the initial rendering but they already show 2 issues.

  • confirm - description is on same line as title (intended?)
  • select/multiselect - do not render options since initialization (capacity/pages) happens in run

src/confirm.rs Outdated Show resolved Hide resolved
src/multiselect.rs Outdated Show resolved Hide resolved
src/input.rs Outdated Show resolved Hide resolved
@jdx
Copy link
Owner

jdx commented Jan 23, 2024

I'll leave the rest of this up to you, but I agree that the title and description shouldn't be on the same line

@@ -133,12 +133,11 @@ impl<'a> Confirm<'a> {
let mut out = Buffer::ansi();

out.set_color(&self.theme.title)?;
write!(out, " {}", self.title)?;
writeln!(out, " {}", self.title)?;
Copy link
Owner

Choose a reason for hiding this comment

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

did you want to remove the 1 column prefix?

I looked at huh and it looks like they actually put a solid line on the side which I don't really like the look of. I don't have a strong preference here, but whatever we do should be consistent across our components I think

Copy link
Owner

Choose a reason for hiding this comment

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

I guess in theory this is something that could be handled via themes if we want to go that route (not sure if huh does that or not)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAIK the line on the side in huh marks the active field in a form/group.

@roele roele merged commit 58c1e51 into jdx:main Jan 23, 2024
1 check passed
@roele roele deleted the issues/tests branch January 23, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants