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

Fix ProjectCard text overflow #395

Merged
merged 4 commits into from Jun 20, 2022
Merged

Fix ProjectCard text overflow #395

merged 4 commits into from Jun 20, 2022

Conversation

AbdulrhmnGhanem
Copy link
Member

@AbdulrhmnGhanem AbdulrhmnGhanem commented May 31, 2022

Fixes #360

before after
image image

@AbdulrhmnGhanem AbdulrhmnGhanem changed the title UI|Fix: ProjectCard text overflow Fix ProjectCard text overflow May 31, 2022
@@ -10,3 +10,7 @@
.card:hover {
text-decoration: none;
}

.cardText {
overflow-wrap: break-word;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
overflow-wrap: break-word;
overflow-wrap: ellipses;

We want the cards to have a consistent height so we can place them in a grid.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it though I don't like it

image

What is the problem with different card sizes? Here is Pinterest for example

image

Copy link
Member

@kasbah kasbah Jun 1, 2022

Choose a reason for hiding this comment

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

I know pinterest does that but personally I think it makes it hard to scan through the list. Fixed width and height project cards in a grid, max 3 per row, is what I had settled on for v1. The description should be given more lines though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Name Description Description when a single word takes multiple lines
image image image

Copy link
Member

Choose a reason for hiding this comment

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

Just looking at these screenshots: the card height should be fixed and not dependent on the number of lines in the description.

@AbdulrhmnGhanem AbdulrhmnGhanem force-pushed the project-card-overflow branch 2 times, most recently from f2ed007 to f8ac04a Compare June 3, 2022 06:27
@kasbah kasbah marked this pull request as ready for review June 6, 2022 12:09
@AbdulrhmnGhanem AbdulrhmnGhanem force-pushed the project-card-overflow branch 2 times, most recently from dd25ca5 to dc4b9f7 Compare June 6, 2022 13:12
@AbdulrhmnGhanem
Copy link
Member Author

AbdulrhmnGhanem commented Jun 6, 2022

In v1 the description text is clipped by trimming the HTML (a preprocessing step?) not handled in CSS

v1 v2
image image

@AbdulrhmnGhanem
Copy link
Member Author

Possible solutions https://css-tricks.com/line-clampin

Comment on lines 24 to 32
display: -webkit-box;
-webkit-line-clamp: 3;
-webkit-box-orient: vertical
}
Copy link
Member

Choose a reason for hiding this comment

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

We agreed we would just switch this to a fixed card height.

@AbdulrhmnGhanem
Copy link
Member Author

AbdulrhmnGhanem commented Jun 9, 2022 via email

@kasbah
Copy link
Member

kasbah commented Jun 9, 2022

Oh, I thought it would. If you want to use box etc. then use the unprefixed versions. Autoprefixer should prefix these for us as needed.

@AbdulrhmnGhanem
Copy link
Member Author

Autoprefixer should prefix these for us as needed.

This isn't the case for this particular one 😅 (postcss/autoprefixer/issues/1322)

@kasbah
Copy link
Member

kasbah commented Jun 18, 2022

Ok, please add a comment about it.

@AbdulrhmnGhanem AbdulrhmnGhanem temporarily deployed to staging June 20, 2022 09:18 Inactive
@kasbah kasbah merged commit 7c62732 into master Jun 20, 2022
@AbdulrhmnGhanem AbdulrhmnGhanem deleted the project-card-overflow branch August 30, 2022 13:30
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.

Project Card UI is breaks when using long names
2 participants