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

Allow showing of games less than 30 seconds #88

Merged
merged 10 commits into from
Aug 17, 2020

Conversation

vinceau
Copy link
Member

@vinceau vinceau commented Aug 11, 2020

Fixes #78

This PR adds the following:

  • a "Click to show all" button which allows the showing of games which have been filtered due to being less than 30 seconds
  • the ability to reveal a file in the default file explorer when clicking on the file name

The setting is stored in the local component state which means the option to filter/show is reset back to the default (which is to filter) whenever the component is unmounted.

Screenshot

screenshot

@JLaferri
Copy link
Member

Trying to think of text that seems more actionable. Like "Click to show all"?

@vinceau
Copy link
Member Author

vinceau commented Aug 11, 2020

Trying to think of text that seems more actionable. Like "Click to show all"?

Isn't "Show" already an action? How's "Show all replays"?

Though all doesn't include corrupted replays. So I don't know if we want to disambiguate that. You could have the situation where it says for example:

9 corrupt files detected. Non-corrupt replays shorter than 30 seconds are automatically filtered. Click to show all

And in that situation it may not be clear that the corrupted files will not actually be shown.

@NikhilNarayana
Copy link
Member

NikhilNarayana commented Aug 11, 2020

I would like to see it as an obvious button, I'm worried users won't realize that text is interactable

@vinceau
Copy link
Member Author

vinceau commented Aug 11, 2020

I would like to see it as an obvious button, I'm worried users won't realize that text is interactable

I intentionally kept the styling minimal since I think you guys would have a much better idea of what you want the button to look like and what kinda aesthetics you're after.

@JLaferri
Copy link
Member

Yeah I think what Nikki mentioned is why I wanted to add the word "Click". But it's a pretty small thing, I'm content enough pushing it as is, UX-wise. I trust you enough to not have to go through the code too deeply lol

@vinceau
Copy link
Member Author

vinceau commented Aug 11, 2020

Changed it to "Click to show all" but I haven't changed the styles, check the updated screenshot. Hopefully it's obvious enough to users but if not feel free to change the styles to something else, Nikki.

@JLaferri
Copy link
Member

What does it look like after you click the button in the case where you:

  1. Didn't have corrupt replays
  2. Did have corrupt replays

@vinceau
Copy link
Member Author

vinceau commented Aug 12, 2020

What does it look like after you click the button in the case where you:

1. Didn't have corrupt replays

2. Did have corrupt replays

In both cases, the entire banner is hidden after the user clicks "Click to show all". In case number 2 we probably want to keep the banner to indicate the number of corrupted replays but the text copy would need to change.

@vinceau
Copy link
Member Author

vinceau commented Aug 17, 2020

Updated the banner to show this in the case that the button has been clicked but there are still corrupted files which were filtered.

screenshot

@NikhilNarayana
Copy link
Member

LGTM

@NikhilNarayana NikhilNarayana merged commit 7f0b626 into project-slippi:master Aug 17, 2020
@vinceau vinceau deleted the feat/toggleable-filter branch August 17, 2020 00:47
@NikhilNarayana NikhilNarayana mentioned this pull request Oct 22, 2020
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.

Add option to change the number of game-seconds that .slp files are filtered by
3 participants