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

Add fixed width/height to PostListing thumbnail parent #1660

Merged
merged 7 commits into from Jun 29, 2023

Conversation

alectrocute
Copy link
Contributor

@alectrocute alectrocute commented Jun 27, 2023

Hi Lemdevs!

I noticed our thumbnails were looking inconsistent, so made an attempt to keep everything 1:1, at 5rem*5rem.

Before:

Screenshot 2023-06-27 at 5 33 20 PM

After:

Screenshot 2023-06-27 at 5 31 08 PM Screenshot 2023-06-27 at 5 31 11 PM

Let me know your thoughts and/or if there's a better way to accomplish this. Our thumbnail generating function is a little bit of a mess, could be a good opportunity for refactor here.

Thanks!

Copy link
Member

@SleeplessOne1917 SleeplessOne1917 left a comment

Choose a reason for hiding this comment

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

LGTM

@jsit
Copy link
Contributor

jsit commented Jun 27, 2023

I think a col-auto next to a col-9 col-sm-10 or whatever it is might not work always. Hold pleaase

@jsit
Copy link
Contributor

jsit commented Jun 27, 2023

Screenshot 2023-06-27 at 6 28 53 PM

@jsit
Copy link
Contributor

jsit commented Jun 27, 2023

Even if they're both col-auto, it doesn't work; the content part can wrap. hold please

Screenshot 2023-06-27 at 6 30 48 PM

Copy link
Contributor

@jsit jsit left a comment

Choose a reason for hiding this comment

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

needs tweaking

alectrocute and others added 2 commits June 27, 2023 19:55
Co-authored-by: Jay Sitter <jsit@users.noreply.github.com>
@alectrocute
Copy link
Contributor Author

Made further tweaks off your work, @jsit.
Screenshot 2023-06-27 at 8 01 05 PM
Screenshot 2023-06-27 at 8 01 10 PM
Screenshot 2023-06-27 at 8 01 14 PM

Screenshot 2023-06-27 at 8 00 55 PM

@alectrocute alectrocute added this to the 0.18.1 milestone Jun 28, 2023
@alectrocute alectrocute requested a review from jsit June 28, 2023 13:38
Copy link
Contributor

@jsit jsit left a comment

Choose a reason for hiding this comment

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

Oops sorry, I forgot to submit this review. Bootstrap already gives us overflow-hidden

src/assets/css/main.css Outdated Show resolved Hide resolved
@alectrocute alectrocute requested a review from jsit June 28, 2023 14:55
@alectrocute
Copy link
Contributor Author

Let's get this merged soon if possible, @dessalines.

@SleeplessOne1917
Copy link
Member

@jsit Your review is still marked as need changes. Is the PR good to go now?

@jsit jsit enabled auto-merge (squash) June 29, 2023 00:04
@dessalines dessalines disabled auto-merge June 29, 2023 02:44
@dessalines dessalines merged commit 38a109b into main Jun 29, 2023
2 checks passed
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

4 participants