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 logo size #606

Merged
merged 1 commit into from
Mar 14, 2024
Merged

Fix logo size #606

merged 1 commit into from
Mar 14, 2024

Conversation

boherm
Copy link
Member

@boherm boherm commented Mar 8, 2024

Questions Answers
Description? After tests review for #600 (review),
we need to fix the logo size when the uploaded image is taller than longer.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Not yet.
Sponsor company PrestaShop SA
How to test? See @florine2623 review in #600 (review)

--

Additional info issue from the release tests

Screenshot 2024-02-23 at 14 59 44

Here's the result ❌

Screen.Recording.2024-02-23.at.15.02.30.mov

templates/_partials/helpers.tpl Outdated Show resolved Hide resolved
tblivet
tblivet previously approved these changes Mar 8, 2024
Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍
If we limit the height, maybe we also need to do the same for the width ?
Like max-width: 150px; ?

@SharakPL
Copy link
Contributor

SharakPL commented Mar 8, 2024

Like max-width: 150px; ?

Yes, but from my experience it should be around 250px

@AureRita AureRita self-assigned this Mar 11, 2024
@AureRita
Copy link

Hi @boherm

Thank you for your PR, currently I tested it and, yes, the logo is a little bit smaller but I think it's not enought as you can see :

image

FYI : you can scroll under the logo

Can you correct it ?

Waiting for your feedback

@AureRita AureRita removed their assignment Mar 11, 2024
@boherm
Copy link
Member Author

boherm commented Mar 13, 2024

Hi @AureRita, thanks for the review!

This is only on the homepage or on a specific page?

It looks like you haven't properly rebuild hummingbird theme or don't clear the browser cache. 🤔
Have you done this steps?

@SharakPL, @tblivet,
So, do you prefer adding max-width: 250px; property and also removing .img-fluid (that is totally bypassed by this rule)?

@boherm boherm force-pushed the fix-logo-size branch 2 times, most recently from c0ca389 to 46bcf66 Compare March 14, 2024 09:06
@SharakPL SharakPL added this to the Beta milestone Mar 14, 2024
@AureRita AureRita self-assigned this Mar 14, 2024
Copy link

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @boherm

Thank you for your PR, I tested it and it seems to works as you can see :

recording.140.webm

Tested on develop and 8.1.x

Because the PR seems to works as expected, It's QA ✔️

Thank you

@SharakPL SharakPL merged commit a5f1476 into PrestaShop:develop Mar 14, 2024
6 checks passed
@boherm boherm deleted the fix-logo-size branch March 15, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants