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

Allow configuring logo dimensions #1237

Open
TheTripleV opened this issue Sep 21, 2021 · 3 comments
Open

Allow configuring logo dimensions #1237

TheTripleV opened this issue Sep 21, 2021 · 3 comments

Comments

@TheTripleV
Copy link

The logo image does not currently specify the width and height or aspect ratio of the logo image. Because of this, the browser cannot reserve space for the image and layout shift occurs.

In layout.html, modifying

{%- if sphinx_version_info < (4, 0) -%}
<img src="{{ pathto('_static/' + logo, 1) }}" class="logo" alt="{{ _('Logo') }}"/>
{%- else %}
<img src="{{ logo_url }}" class="logo" alt="{{ _('Logo') }}"/>
{%- endif %}

to

<picture>
    {%- if sphinx_version_info < (4, 0) -%}
    <img src="{{ pathto('_static/' + logo, 1) }}" class="logo" alt="{{ _('Logo') }}" width="{{ logo_width }}" height="{{ logo_height }}"/>
    {%- else %}
    <img src="{{ logo_url }}" class="logo" alt="{{ _('Logo') }}" width="{{ logo_width }}" height="{{ logo_height }}"/>
    {%- endif %}
</picture>

would decrease cumulative layout shift.

This example solution requires 2 additional configuration options to be added.

@nienn
Copy link
Contributor

nienn commented Sep 21, 2021

Hi @TheTripleV, your solution, although it follows the html best practices, is limitative to big portion of users.

Currently, the logo width is set either by the real image width or by the css max-width: 100% rule, and the height is set accordingly by the browser, with respect to the real image aspect ratio.

Forcing a fixed unit width and height in this manner would provoke image distortions to many users.

The layout shift is definitely something to keep in mind thought and we'll try to look for a solution in future releases that might be less imposing to users.

For now, you can still add a fixed width and height to your logo with a custom css.

@TheTripleV
Copy link
Author

I don't think image distortion will occur. If I am understanding correctly, when an <img> is nested in a <picture>, the <picture> width and height control the dimensions of the logo on screen. The <img> width and height are used to determine the placeholder size needed.

So, the full snippet would be closer to:

<picture width="PREVIOUS_IMG_WIDTH", height="PREVIOUS_IMG_HEIGHT">
    {%- if sphinx_version_info < (4, 0) -%}
    <img src="{{ pathto('_static/' + logo, 1) }}" class="logo" alt="{{ _('Logo') }}" width="{{ src_wdith }}" height="{{ src_height }}"/>
    {%- else %}
    <img src="{{ logo_url }}" class="logo" alt="{{ _('Logo') }}" width="{{ src_wdith }}" height="{{ src_height }}"/>
    {%- endif %}
</picture>

The old <img> width and height (if any) get applied to the <picture> instead.
The new <img> width and height becomes the the dimensions of the src file or link.

@nienn
Copy link
Contributor

nienn commented Sep 21, 2021

Ah yes, sorry, I didn't notice the picture tag.
For the moment however, we are still supporting html 4 and IE11 so using <picture> is still not an option.
We will be able to do this on the next major release thought!

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

No branches or pull requests

2 participants