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

added back button #66

Merged
merged 9 commits into from
Apr 14, 2024
Merged

added back button #66

merged 9 commits into from
Apr 14, 2024

Conversation

suvanbanerjee
Copy link
Contributor

@suvanbanerjee suvanbanerjee commented Apr 10, 2024

added back button in first three pages. i tested it and is working i just want to know is doing it this way works ? or i am missing something.
Fixes #64

@suvanbanerjee
Copy link
Contributor Author

suvanbanerjee commented Apr 10, 2024

@goldyfruit please check

@goldyfruit goldyfruit added the enhancement New feature or request label Apr 10, 2024
Copy link
Member

@goldyfruit goldyfruit left a comment

Choose a reason for hiding this comment

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

Make sure to add BACK_BUTTON the to other locales as well.

tui/channels.sh Outdated Show resolved Hide resolved
tui/channels.sh Outdated Show resolved Hide resolved
tui/locales/en-us/misc.sh Outdated Show resolved Hide resolved
tui/methods.sh Outdated Show resolved Hide resolved
tui/profiles.sh Outdated Show resolved Hide resolved
tui/methods.sh Outdated Show resolved Hide resolved
tui/profiles.sh Outdated Show resolved Hide resolved
@goldyfruit
Copy link
Member

goldyfruit commented Apr 11, 2024

Just tested the PR, and I don't think this will be the best approach as I'm pretty sure users will not see the Back button (which is not a button indeed)

image

Maybe we should just replace the Cancel button by the Back button.

@mikejgray @JarbasAl what do you think?

@goldyfruit goldyfruit marked this pull request as ready for review April 11, 2024 00:43
@mikejgray
Copy link

Just tested the PR, and I don't think this will be the best approach as I'm pretty sure users will not see the Back button (which is not a button indeed)

image

Maybe we should just replace the Cancel button by the Back button.

@mikejgray @JarbasAl what do you think?

I agree, this is confusing because it looks like Back is a branch.

If you replace Cancel with Back, can you still close out the TUI with Control+C?

@suvanbanerjee
Copy link
Contributor Author

suvanbanerjee commented Apr 11, 2024

@goldyfruit
I will replace the Cancle with Back.
@mikejgray Ctrl+C will not close TUI. Ctrl+C now also don't closes the TUI. We have to use ESC to quit the TUI but ESC to quit dont works on Fedora/CentOS (https://superuser.com/questions/966589/esc-key-does-not-work-for-whiptail-on-centos-fedora)
I think is due to limitations of whiptail one solution should be moving to dialogs but that will be a major code refactor.

@suvanbanerjee
Copy link
Contributor Author

suvanbanerjee commented Apr 11, 2024

@goldyfruit i have change Cancel button to Back in first 3 panels. If it looks good then i will also update the rest of a pages. Please allow me a couple of days because after profiles there are multiple paths so it will take time to add back buttons on those pages

image

@goldyfruit
Copy link
Member

Yeah looks better. 👍

@suvanbanerjee
Copy link
Contributor Author

suvanbanerjee commented Apr 13, 2024

@goldyfruit Please check I have replaced the cancel button by back button in all pages. I have to do some major changes in satellite\main.sh please test it extensively

@suvanbanerjee suvanbanerjee changed the title added back button in some pages added back button Apr 13, 2024
Copy link
Member

@goldyfruit goldyfruit left a comment

Choose a reason for hiding this comment

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

CANCEL_BUTTON can be removed from the locale files.

tui/features.sh Outdated Show resolved Hide resolved
tui/features.sh Outdated Show resolved Hide resolved
tui/methods.sh Outdated Show resolved Hide resolved
tui/satellite/host.sh Outdated Show resolved Hide resolved
tui/satellite/host.sh Outdated Show resolved Hide resolved
tui/tuning.sh Outdated Show resolved Hide resolved
tui/satellite/port.sh Outdated Show resolved Hide resolved
tui/satellite/password.sh Outdated Show resolved Hide resolved
tui/satellite/password.sh Outdated Show resolved Hide resolved
tui/tuning.sh Show resolved Hide resolved
@suvanbanerjee
Copy link
Contributor Author

suvanbanerjee commented Apr 14, 2024

CANCEL_BUTTON can be removed from the locale files.

I think we should not because in future there is a chance we might need a cancel button. However, if you prefer, I will remove it.

@goldyfruit
Copy link
Member

CANCEL_BUTTON can be removed from the locale files.

I think we should not because in future there is a chance we might need a cancel button. However, if you prefer, I will remove it.

I would remove it since we saw that two buttons is the only "non-confusing" option.

@goldyfruit goldyfruit merged commit ad72122 into OpenVoiceOS:main Apr 14, 2024
@goldyfruit
Copy link
Member

Good job @suvanbanerjee 👍 🎉

@suvanbanerjee
Copy link
Contributor Author

Thanks @goldyfruit for your mentorship. many more PR to go 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Back button in TUI
3 participants