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

Rethink [confirm] component theme configuration #2034

Closed
2 tasks done
uncenter opened this issue Dec 12, 2024 · 6 comments
Closed
2 tasks done

Rethink [confirm] component theme configuration #2034

uncenter opened this issue Dec 12, 2024 · 6 comments
Labels
feature New feature request

Comments

@uncenter
Copy link
Contributor

uncenter commented Dec 12, 2024

Please describe the problem you're trying to solve

Rename btn_yes and btn_no to btn_active and btn_inactive, and support toggling between the confirm actions ("yes" and "no") with the Tab key or arrows keys.

btn_labels option also probably shouldn't be at the theme level, as it doesn't have much to do with colors and such. Looks like there is already a [confirm] section in yazi.toml that it can go under.

Checklist

  • I have searched the existing issues/discussions
  • The latest nightly build doesn't already have this feature
@uncenter uncenter added the feature New feature request label Dec 12, 2024
@sxyazi
Copy link
Owner

sxyazi commented Dec 12, 2024

support toggling between the confirm actions ("yes" and "no") with the Tab key or arrows keys

If the user wants to cancel the confirmation, i.e. choose No, they first need to use Tab to switch from the default Yes to No, and then press Enter.

This requires two steps. Why not just press n or Esc to cancel, which only requires one step?

What's the point of adding an activation state switch? What problem does it solve?

btn_labels option also probably shouldn't be at the theme level, as it doesn't have much to do with colors and such.

You're right, it should be in yazi.toml not theme.toml, can't remember why I put it in theme.toml :/

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Dec 12, 2024
@uncenter
Copy link
Contributor Author

uncenter commented Dec 12, 2024

I think the primary point of confusion is that the confirm component seems to be doing two things. It's acting like a y/n prompt, while looking like it has multiple options. For example, if I change the button labels to True and False, I assume I would still need to do y/n, despite that not matching the labels. Especially with the default theme styling which inverts the yes option, it looks like it should be toggleable. Maybe removing the invert would help clarify?

@github-actions github-actions bot removed the waiting on op Waiting for more information from the original poster label Dec 12, 2024
@sxyazi
Copy link
Owner

sxyazi commented Dec 12, 2024

For example, if I change the button labels to True and False, I assume I would still need to do y/n, despite that not matching the labels

If you change the button labels to True and False, then you should also change the keys to t and f; it's entirely up to you:

{ on = "n", run = "close", desc = "Cancel the confirm" },
{ on = "y", run = "close --submit", desc = "Submit the confirm" },

Especially with the default theme styling which inverts the yes option, it looks like it should be toggleable. Maybe removing the invert would help clarify?

The inversion was added to let the user know what the default behavior is. It's more of a safety measure, not necessarily indicating that it can actually be toggled.

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Dec 12, 2024
@uncenter
Copy link
Contributor Author

Gotcha. Maybe make it bold then so it looks less like a button?

@github-actions github-actions bot removed the waiting on op Waiting for more information from the original poster label Dec 12, 2024
@sxyazi
Copy link
Owner

sxyazi commented Dec 12, 2024

I thought about it again, and I think it's fine for btn_labels to be included in the theme, because it's used to control the button content, and the content itself is part of the style.

For example, users can remove spaces from the default content to make the button more compact:

btn_labels = [ " [Y]es ", " (N)o " ]

yazi.toml should only be used to control the actual behavior, and shouldn't include any information about colors, titles, positions, etc.

That means the existing [input], [confirm], and [pick] should also be moved to theme.toml, so users can modify all styles in one place without having to jump between multiple files:

[input]
cursor_blink = false
# cd
cd_title = "Change directory:"
cd_origin = "top-center"
cd_offset = [ 0, 2, 50, 3 ]
# create
create_title = [ "Create:", "Create (dir):" ]
create_origin = "top-center"
create_offset = [ 0, 2, 50, 3 ]
# rename
rename_title = "Rename:"
rename_origin = "hovered"
rename_offset = [ 0, 1, 50, 3 ]
# filter
filter_title = "Filter:"
filter_origin = "top-center"
filter_offset = [ 0, 2, 50, 3 ]
# find
find_title = [ "Find next:", "Find previous:" ]
find_origin = "top-center"
find_offset = [ 0, 2, 50, 3 ]
# search
search_title = "Search via {n}:"
search_origin = "top-center"
search_offset = [ 0, 2, 50, 3 ]
# shell
shell_title = [ "Shell:", "Shell (block):" ]
shell_origin = "top-center"
shell_offset = [ 0, 2, 50, 3 ]
[confirm]
# trash
trash_title = "Trash {n} selected file{s}?"
trash_origin = "center"
trash_offset = [ 0, 0, 70, 20 ]
# delete
delete_title = "Permanently delete {n} selected file{s}?"
delete_origin = "center"
delete_offset = [ 0, 0, 70, 20 ]
# overwrite
overwrite_title = "Overwrite file?"
overwrite_content = "Will overwrite the following file:"
overwrite_origin = "center"
overwrite_offset = [ 0, 0, 50, 15 ]
# quit
quit_title = "Quit?"
quit_content = "The following task is still running, are you sure you want to quit?"
quit_origin = "center"
quit_offset = [ 0, 0, 50, 15 ]
[pick]
open_title = "Open with:"
open_origin = "hovered"
open_offset = [ 0, 1, 50, 7 ]

The reason they were previously placed in yazi.toml is because back then flavors weren't supported, and users could only use theme.toml.

If they added custom styles for these components in theme.toml, it would cause conflicts during updates (see #351 (comment)), but now, that's no longer an issue.

@sxyazi sxyazi mentioned this issue Jan 2, 2025
2 tasks
@sxyazi
Copy link
Owner

sxyazi commented Jan 2, 2025

Added to v0.5 todo list since it requires breaking changes

@sxyazi sxyazi closed this as completed Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request
Projects
None yet
Development

No branches or pull requests

2 participants