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

hosts page - incorrect layout after photo change #377

Closed
gerbrent opened this issue Sep 2, 2022 · 7 comments · Fixed by #397
Closed

hosts page - incorrect layout after photo change #377

gerbrent opened this issue Sep 2, 2022 · 7 comments · Fixed by #397
Labels
bug Something isn't working design design and visual

Comments

@gerbrent
Copy link
Collaborator

gerbrent commented Sep 2, 2022

Alex changed his profile photo in #373
This seems to have caused layout issues

image

@ironicbadger can you tell us where you got the photo? I presume you copied it from some other online profile where the image has been shrunk.

My theory:
I presume given it's data size - 29 KB - and it's pixel dimensions - 300x263 - that a too-small image size applied to a profile causes layout problems instead of being scaled/interpolated (despite pixelation).

@ironicbadger can you procure a higher-resolution copy relatively easily? Then again, this may not be the solution, but is a bandaid...

@gerbrent gerbrent added bug Something isn't working design design and visual labels Sep 2, 2022
@elreydetoda
Copy link
Collaborator

@ChanceM (guessing that's them, since same name in matrix) posted this:

Looking at issue #377, which led down a deeper rabbit hole. Do you know why hosts/guests are not rendered with the people layout? Right now they are rendered with the default list layout, not sure if they was by design.

my brief response (more later):

tl;dr: IIRC since there is no content in the content/hosts/ (& guests) then there is no way to apply a list page to it besides through the default list page (at least from what I could tell)

So, that's why I check for hosts/guests

Plus the people's page lists both out, so it's got an extra range for the groups of people.

@ChanceM
Copy link
Contributor

ChanceM commented Sep 6, 2022

@ChanceM (guessing that's them, since same name in matrix) posted this:

Looking at issue #377, which led down a deeper rabbit hole. Do you know why hosts/guests are not rendered with the people layout? Right now they are rendered with the default list layout, not sure if they was by design.

my brief response (more later):

tl;dr: IIRC since there is no content in the content/hosts/ (& guests) then there is no way to apply a list page to it besides through the default list page (at least from what I could tell)

So, that's why I check for hosts/guests

Plus the people's page lists both out, so it's got an extra range for the groups of people.

Okay changing the type from host to people in the content/hosts/_index.md will make it render with the people template and adding the same for guests will do the same.

For the duplicate of a host as a guest we would need to duplicate the person but set the type to guest.

@ChanceM
Copy link
Contributor

ChanceM commented Sep 6, 2022

@elreydetoda, @gerbrent I have a working version in a branch. It would require some rework for those that are hosts and guests but it would limit the need to retype contact info twice.

@elreydetoda
Copy link
Collaborator

Hey @ChanceM , I wasn't positive about this earlier but I was just able to test it out locally. It seems like you're trying to solve #376 and this issue with your branch.

While I'm all for trying to get the most results with a PR, I think in this instance we might want to split them up. #376 (just off the top of my head) will require at minimum scraper changes. While they might seem insignificant the scraper isn't the most modular so it'll take some time.

For this issue though, you already have everything you need to close it. It's just the styling changes you made to/for the cards already. I was able to cherry-pick 2 of your commits and then replicated the only other styling change you'd need on this branch: https://github.com/elreydetoda/jupiterbroadcasting.com/tree/just_astetics

If you could just do a PR with only the changes you made for aesthetics (i.e. what I picked/replicated in my branch), then we could definitely close this issue with that PR.

image

Thank you for taking the time to figure this out 🥳 , and I'll comment on #376 about some of the questions I have about your approach for solving that issue (i.e. the rest of what you were suggesting with the branch you'd mentioned)

@ChanceM
Copy link
Contributor

ChanceM commented Sep 7, 2022

Well part of those changes incorporating fixing it to use the correct template. Should that portion be incorporated in #376 or into a new issue?

@elreydetoda
Copy link
Collaborator

yes, that should be in #376

closed with #397

@gerbrent
Copy link
Collaborator Author

gerbrent commented Sep 8, 2022

For future reference:

most of the images were scraped by the scraper from Fireside. The images thus were necessarily previously ingested by Fireside's process and likely made to conform to a specific standard.

Alex' photo is the first to be replaced independent of Fireside - he applied it here as a PR - and thus is the first case of integrating non-standard photos into our site here.

THAT is the difference, and why all a sudden Alex' photo is problematic.

Likely we will need to test a variety of images to see how Hugo & company manage their integration seamlessly.

Originally posted by @gerbrent in #409 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design design and visual
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants