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

Truncated description on game cards and decreased size of description on game page #288

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

willcravitz
Copy link
Contributor

@willcravitz willcravitz commented Nov 25, 2023

This code does 2 things to address issue #258:

  1. Truncates the description to 20 words on game cards on the game grid page
  2. On the game detail page, decreases the size of the description (from h3 to p) and moves it into the information section

@willcravitz willcravitz self-assigned this Nov 25, 2023
@willcravitz willcravitz linked an issue Nov 25, 2023 that may be closed by this pull request
@willcravitz willcravitz added this to the 2023/Sprint 3 milestone Nov 25, 2023
@willcravitz willcravitz requested review from majorsylvie, frowenz and danielngira and removed request for frowenz November 25, 2023 23:48
Copy link
Contributor

@danielngira danielngira left a comment

Choose a reason for hiding this comment

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

The code seems correct and achieves the intended purpose. I left two comments, one is a minor one on style and the other one was me contemplating how to handle the description on the grid view page. The changes I suggested are optional really so I'll approve the PR once you contemplate them

@@ -54,7 +54,7 @@ <h1 class="display-1 mt-3 mb-3">Games</h1>
</div>
<div class="card-body d-flex flex-column">
<h4 class="card-title">{{ game.name }}</h4>
<p class="card-text flex-grow-1">{{ game.description|linebreaksbr }}</p>
<p class="card-text flex-grow-1">{{ game.description|linebreaksbr|truncatewords_html:20 }}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

image

The description in the game grid looks perfect! I was thinking, is it possible to let the user know/sth that can signal to them that they can read more about the game description so they can know the game description was purposely truncated and there's a complete version of it? Can a "read more" under the description be a useful indicator or would it be redundant since the whole grid itself is a link to the game detail page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to what I said in the comment above, I think this is a really good suggestion, but I'd rather leave it up to future team members to open up a new game card styling issue. The main focus of this issue was to make sure that descriptions don't make the cards too big.

Copy link
Contributor

Choose a reason for hiding this comment

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

Like with the previous comment, I think your judgement makes sense. I can resolve these comments now

@@ -54,7 +54,7 @@ <h1 class="display-1 mt-3 mb-3">Games</h1>
</div>
<div class="card-body d-flex flex-column">
<h4 class="card-title">{{ game.name }}</h4>
<p class="card-text flex-grow-1">{{ game.description|linebreaksbr }}</p>
<p class="card-text flex-grow-1">{{ game.description|linebreaksbr|truncatewords_html:20 }}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Like with the previous comment, I think your judgement makes sense. I can resolve these comments now

Copy link
Contributor

@majorsylvie majorsylvie left a comment

Choose a reason for hiding this comment

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

Looks great! The Grid view is significantly cleaner and more organized. Thank you for adding the ... to signal that there is more to read.

Similarly the detail view feels even more put together now that the size has been meaningfully decreased.

This code is good to merge. I do have two further suggestions on more changes to make!

I would recommend changing the alignment of detail view elements to address the problem of (particularly large vertical) images creating whitespace above the description:
image

I agree that a great next step would be adding a Read More button / link to expand a given card.

An idea for that is to have it bring up the card in a "center peek" view. Notion does this for example:
image

@majorsylvie majorsylvie merged commit 1013f5a into dev Nov 27, 2023
2 checks passed
@majorsylvie majorsylvie deleted the game-management/descriptions branch November 27, 2023 15:24
@willcravitz
Copy link
Contributor Author

willcravitz commented Nov 27, 2023

Closing comments: The descriptions on game cards are now limited to 20 words followed by an ellipses. Descriptions on the detail page took up too much space as a header, so they are now paragraph text.

@danielngira and @majorsylvie both suggested ways to further improve the user experience on both the game grid and detail pages which should be addressed in future issues.

@majorsylvie
Copy link
Contributor

Issue Score: Excellent

Comments:
Thank you @danielngira for the good review and offering suggestions
@willcravitz great work! Appreciate the good closing comment as well as the list of things done by this PR!

Keep moving forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix the way that game descriptions are displayed
3 participants