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

Ani-cli UX Changes #1019

Merged
merged 5 commits into from
Feb 5, 2023
Merged

Ani-cli UX Changes #1019

merged 5 commits into from
Feb 5, 2023

Conversation

athuld
Copy link
Contributor

@athuld athuld commented Feb 4, 2023

Pull Request Template

Type of change

  • Bug fix
  • Feature
  • Documentation update

Description

Hi guys just want to fix UX related issues

  • the episodes are displayed to descending order which is usually not a good experience for users
  • the menu should display the next episode option first followed by other relevant option and currently the quit option is displayed first after each episode play ends

PS: don't have syncplay so didn't test that.. apart from that everything is working

Checklist

  • any anime playing
  • bumped version

  • next, prev and replay work
  • -c history and continue work
  • -d downloads work
  • -s syncplay works
  • -q quality works
  • -v vlc works
  • -e select episode works
  • -r range selection works
  • --dub both work
  • all providers return links (not necessarily on a single anime, use debug mode to confirm)

  • -h help info is up to date
  • Readme is up to date
  • Man page is up to date

Additional Testcases

  • The safe bet: One Piece
  • Episode 0: Saenai Heroine no Sodatekata ♭
  • Unicode: Saenai Heroine no Sodatekata ♭
  • Non-whole episodes: Tensei shitara slime datta ken (ep. 24.5, ep. 24.9)

* the current implementation shows the episode numbers in asc order
  which is not really good for ux

Signed-off-by: Athul Dinesan <athuldinesan5@gmail.com>
* users usually want to use the next episode option instead of the quit
  option. so reordering the menu to be more inline with user choices

Signed-off-by: Athul Dinesan <athuldinesan5@gmail.com>
Signed-off-by: Athul Dinesan <athuldinesan5@gmail.com>
@athuld athuld requested a review from port19x as a code owner February 4, 2023 19:15
Copy link
Collaborator

@port19x port19x left a comment

Choose a reason for hiding this comment

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

I agree that next should be the default option in the interactive menu.
I'm however uncertain that an ascending order makes more sense.
Viewers of airing shows for example might only be interested in the latest, tho since those can and should use -c that still leaves your stance valid.
I'll make a poll

@athuld
Copy link
Contributor Author

athuld commented Feb 4, 2023

As with the case of new users seeing the last episode of the anime as the first option to select might not be a good experience and even though you can scroll up the fzf to reach the first episode, most users don't know this and by default scrolls down. This is usually not an ideal way to navigate the episodes in my opinion. Given an ordered list new users can easy go through the episode and viewers of airing shows usually uses -c option or since most anime are usuallyy released in seasons .. making the order ascending seems to be the better choice

@port19x
Copy link
Collaborator

port19x commented Feb 4, 2023

As with the case of new users seeing the last episode of the anime as the first option to select might not be a good experience and even though you can scroll up the fzf to reach the first episode, most users don't know this and by default scrolls down. This is usually not an ideal way to navigate the episodes in my opinion. Given an ordered list new users can easy go through the episode and viewers of airing shows usually uses -c option or since most anime are usuallyy released in seasons .. making the order ascending seems to be the better choice

I always assumed the default behaviour to be hitting 1 RET
But yeah, makes sense I guess now that you put it like that

@port19x
Copy link
Collaborator

port19x commented Feb 5, 2023

Alright then
2023-02-05_12-53

@port19x port19x merged commit b268d6c into pystardust:master Feb 5, 2023
@port19x
Copy link
Collaborator

port19x commented Feb 5, 2023

My god, why was the executable bit removed, do you have brain damage?
Rip master

@justchokingaround
Copy link
Collaborator

xdddddd

@athuld
Copy link
Contributor Author

athuld commented Feb 5, 2023

My god, why was the executable bit removed, do you have brain damage? Rip master

My bad sorry I was testing it with user permission..I might have never noticed it..sorry

@athuld
Copy link
Contributor Author

athuld commented Feb 6, 2023

419859c

@port19x this is what caused the bit change.It was not my mistake...next time instead of blindly calling someone brain dead you should check the facts

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.

3 participants