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 favorite button in Performer grid view #3369

Merged
merged 5 commits into from
Feb 17, 2023

Conversation

CrookedDuck
Copy link
Contributor

Resolves part of #705
By clicking the favorite icon the user can toggle whether or not the performer is favorite.

@kermieisinthehouse
Copy link
Collaborator

Thank your for your contribution! I'd love to merge this but there are a few outstanding comments:

  • The state after clicking the icon is ambiguous, it is solid white both after favoriting and unfavoriting. (Tested on Firefox Mobile)
  • For the unfavorited state, I believe the shadow should be less dark, and perhaps raise to the current level of darkness on hover.
  • There are some lint errors that need to be fixed before CI passes.

Thanks! Feel free to comment with any questions.

@DogmaDragon
Copy link
Collaborator

Would love a show/hide toggle to be added in the settings. I see it adds a unique class so it can be hidden via CSS, but a toggle would be more user-friendly.

Copy link
Collaborator

@cj12312021 cj12312021 left a comment

Choose a reason for hiding this comment

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

The usability of this button on mobile should be improved by removing the hover effects on the bottom. Currently, 2 clicks are required to favorite a performer. 1 click to register the hover effect, and another to actually click the button.

ui/v2.5/src/components/Performers/PerformerCard.tsx Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
ui/v2.5/src/components/Performers/styles.scss Outdated Show resolved Hide resolved
@@ -85,6 +85,14 @@
width: 3rem;
}

.not-favorite {
color: rgba(191, 204, 214, 0.5);
filter: drop-shadow(0 0 2px rgba(0, 0, 0, 0.9));
Copy link
Collaborator

Choose a reason for hiding this comment

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

A 50% opacity shadow should be enough here.

@cj12312021
Copy link
Collaborator

Thanks for your contribution! In the future, it would make tester's lives easier if you don't push changes directly to the develop branch.

@kermieisinthehouse
Copy link
Collaborator

Thanks for your contribution! In the future, it would make tester's lives easier if you don't push changes directly to the develop branch.

I'm not sure I understand what this means?

@cj12312021
Copy link
Collaborator

I essentially had to delete the develop branch I had locally to check out his changes since it was pushed directly to the develop branch on his fork. It's not a huge deal but it was an inconvenience.

@kermieisinthehouse
Copy link
Collaborator

Ah, I see. I put the following in my .gitconfig,

[alias]
        pr = "!f() { if [ $# -lt 1 ]; then echo \"Usage: git pr <id> [<remote>]  # assuming <remote>[=origin] is on GitHub\"; else git checkout -q \"$(git rev-parse --verify HEAD)\" && git fetch -fv \"${2:-origin}\" pull/\"$1\"/head:pr/\"$1\" && git checkout pr/\"$1\"; fi; }; f"

So that I can do git pr 3369 and check out a new branch - then build the docker images from there.

@cj12312021
Copy link
Collaborator

Sounds like that would save me some trouble. I'll need to try it out. Thanks!

@cj12312021 cj12312021 added feature Pull requests that add a new feature ui Issues related to UI labels Jan 29, 2023
@CrookedDuck
Copy link
Contributor Author

Thanks for your contribution! In the future, it would make tester's lives easier if you don't push changes directly to the develop branch.

ill keep in mind to create feature/issue related branches in the future, sorry for the inconvenience ;)

@WithoutPants WithoutPants added this to the Version 0.20.0 milestone Feb 16, 2023
@WithoutPants
Copy link
Collaborator

Fixed up the compile errors and changed the styling to be consistent with the older version. Unfavorited performers now only show the favorite button when the performer card is hovered over, since having all the buttons shown was pretty ugly.

@WithoutPants WithoutPants merged commit bb6fa04 into stashapp:develop Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull requests that add a new feature ui Issues related to UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants