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: custom styles for the confirm component #1789

Merged
merged 6 commits into from
Oct 17, 2024

Conversation

GrzegorzKozub
Copy link
Contributor

@GrzegorzKozub GrzegorzKozub commented Oct 16, 2024

While preserving the original design if we don't provide any config, this change let's us customize the confirm component further by:

  • hiding the separators and the scrollbar
  • changing the button text
  • changing the border, title, content, list and button styles

The default design looks like this:

image

And we can achieve a more minimalistic look that fits the color scheme with the following config:

image

[confirm]
show_separators = false
show_scrollbar = false
button_yes = "Yes"
button_no = "No"
border = { fg = "darkgray" }
title = { fg = "darkgray" }
content = { fg = "gray" }
list = { fg = "reset" }
buttons = { fg = "gray" }

What do you think?

I'm not sure how to add the new config to the schema.

@GrzegorzKozub GrzegorzKozub marked this pull request as ready for review October 16, 2024 06:23
Copy link
Owner

@sxyazi sxyazi left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. Here are a few change requests:

  • The "no" button shouldn't use the same style as the "yes" button (i.e., reversed = true). This reduces distinction between the two, making it hard to identify the current selection and potentially introducing safety risks.
  • There should be a way to style the "no" and "yes" buttons separately, rather than applying the same style to both.
  • Please merge the two labels into a single option, like this: [" [Y]es ", " (N)o "].
  • Please remove show_scrollbar and show_separators. Scrollbars only appear when needed (i.e., when the content is long enough to require scrolling), so disabling them isn't very meaningful. As for show_separators, it reduces clarity between the content and buttons and isn't visually appealing — this will also make it harder to distinguish once ya.confirm() API is introduced since it will allow arbitrary styles and alignment of its content.

@GrzegorzKozub
Copy link
Contributor Author

Hopefully all your requests are now satisfied @sxyazi. This brought us back to the correct default:

image

While still allowing us to customize to fit our themes:

[confirm]
button_labels = ["Yes", "No"]
border = { fg = "darkgray" }
title = { fg = "darkgray" }
content = { fg = "gray" }
list = { fg = "reset" }
button_yes = { fg = "gray" }
button_no = { fg = "darkgray" }

image

@sxyazi sxyazi force-pushed the confirm-customization branch from 5df499d to 79b4304 Compare October 17, 2024 10:29
Copy link
Owner

@sxyazi sxyazi left a comment

Choose a reason for hiding this comment

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

All looks good to me now, thank you so much for contributing this feature @GrzegorzKozub!

@sxyazi sxyazi changed the title feat: confirm component customization feat: custom styles for the confirm component Oct 17, 2024
@sxyazi sxyazi merged commit 5304331 into sxyazi:main Oct 17, 2024
6 checks passed
@GrzegorzKozub
Copy link
Contributor Author

Thank you!

@GrzegorzKozub GrzegorzKozub deleted the confirm-customization branch October 17, 2024 11:12
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants