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

[UI] Hide game title when the card is not focused #1569

Merged
merged 8 commits into from
Jul 17, 2022
Merged

Conversation

flavioislima
Copy link
Member

@flavioislima flavioislima commented Jul 12, 2022

This brings the old behavior for the game title. Instead of showing the title all the time, it will be shown only on hover/focus.


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima flavioislima changed the base branch from main to beta July 12, 2022 19:47
@CommandMC
Copy link
Collaborator

Thanks, I think most people (including myself) prefer this design.

I have 4 ideas relating to this:

  1. The title is a little bit high up now, seems like it's spanning half the game image now. I'd say that's a little too much:
    image
  2. Not sure if it was introduced with this PR, but just a little oddity I've noticed: The game image is cut off by the settings/play icons:
    image
  3. Would it be possible to hide the settings/play icons as well, so it's just the image?
  4. If possible, adding a setting to control this behavior would be great. Maybe even have the distinction between "Hide title"/"Hide title and buttons"/"Hide nothing" there?

@flavioislima
Copy link
Member Author

Thanks, I think most people (including myself) prefer this design.

I have 4 ideas relating to this:

  1. The title is a little bit high up now, seems like it's spanning half the game image now. I'd say that's a little too much:
    image
  2. Not sure if it was introduced with this PR, but just a little oddity I've noticed: The game image is cut off by the settings/play icons:
    image
  3. Would it be possible to hide the settings/play icons as well, so it's just the image?
  4. If possible, adding a setting to control this behavior would be great. Maybe even have the distinction between "Hide title"/"Hide title and buttons"/"Hide nothing" there?
  1. Yeah I can try that
  2. I believe it was after I changed the design or after the PR that changed the size of the cards to make them use the whole space.
  3. Then it's like we have it on Stable. I think having the icons is a good thing since with it you have the info already that you have an update, or the game is installed or not. these changes were made also to improve the accessibility,
  4. I think that's too much, as more settings we add more complicated things become to end-user and for us to maintain. I think we should avoid settings for the UI.

Bottom line is that most people liked the new changes, except for the title being shown all the time, at least this was the feedback we got on twitter and on discord.

@CommandMC
Copy link
Collaborator

Ah, fair enough. If it's what the people want, do it

Well, as soon as those two issues (1. and 2.) are fixed, I'm happy with this

@arielj
Copy link
Collaborator

arielj commented Jul 13, 2022

@flavioislima can I push a commit here to fix issues 1 and 2 reported by CommandMC?

@flavioislima
Copy link
Member Author

@flavioislima can I push a commit here to fix issues 1 and 2 reported by CommandMC?

Just saw your message now.
I sent I few fixes, don't know if they are really good.
If you find a better solution feel free to send a commit :D

@arielj
Copy link
Collaborator

arielj commented Jul 14, 2022

@flavioislima can I push a commit here to fix issues 1 and 2 reported by CommandMC?

Just saw your message now. I sent I few fixes, don't know if they are really good. If you find a better solution feel free to send a commit :D

Done! I removed some fixed numbers and calcs. I also removed the limit of 4 lines of text for the title, the longest title I found was tiny tina's and it was 5 lines, they fit just fine inside the card, I don't think we need that limit

@flavioislima flavioislima merged commit 3dcc9a9 into beta Jul 17, 2022
@flavioislima flavioislima deleted the fix/card_style branch July 17, 2022 10:39
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.

None yet

3 participants