Update settings for header section #198
Conversation
t-kelly
requested review from
chrisberthe,
NathanPJF,
huguestennier and
MaxMusing
src/sections/header.liquid
Outdated
| "min": 50, | ||
| "max": 450, | ||
| "step": 8, | ||
| "unit": "px", | ||
| "label": "Custom logo width (in pixels)", |
There was a problem hiding this comment.
Units are there, so no need for (in pixels).
src/sections/header.liquid
Outdated
| "id": "logo_max_width", | ||
| "min": 50, | ||
| "max": 450, | ||
| "step": 8, |
There was a problem hiding this comment.
Is 8 a generic value you chose or one of our standards now?
There was a problem hiding this comment.
Is there another theme that is using the range selector for the logo yet? My logic was (450 - 50) = 400 / 8 = 50px increments
There was a problem hiding this comment.
No, it seems like there aren't any yet for headers. I understood how you got 8 but was just wondering if we had established a pattern for this. Bump it to 10? 😛
Do we want to stretch the logo? Isn't that what would happen if we added |
|
Not sure, realistically we wouldn't want to stretch the image. I just found it weird that with a smaller image it caps out at its actual width even if I'm selecting "400px". I'd be curious what others think. |
|
|
I feel like we should let it resize bigger even if it gets blurry because I wouldn't want the range slider to not work at some value since the image has a max-width. The merchant can always upload a better image. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
What are you trying to accomplish with this PR?
Fixes #172
For maintainers:
package.jsonversion in a separate PR, if applicable.