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

srcset missing for header images in new editor #17753

Closed
1 task done
cathysarisky opened this issue Aug 17, 2023 · 4 comments · Fixed by TryGhost/Koenig#1074
Closed
1 task done

srcset missing for header images in new editor #17753

cathysarisky opened this issue Aug 17, 2023 · 4 comments · Fixed by TryGhost/Koenig#1074
Assignees
Labels
affects:editor Work relating to the Koenig Editor bug [triage] something behaving unexpectedly p2:major [triage] Bugs which we'll fix soon

Comments

@cathysarisky
Copy link
Contributor

Issue Summary

Image content generated for the new split header layout does not include a srcset. Huge images load by default.

Steps to Reproduce

Create a new page with a header card with the new split layout.
Be lazy and drag a huge image into the image blog, assuming Ghost will resize it.
Publish. Inspect the result. Here's the outcome:

<div class="kg-header-card-content">
                
        <picture><img class="kg-header-card-image" src="https://www.spectralwebservices.com/content/images/2023/08/ghost-website-1.png" alt=""></picture>
    
                <div class="kg-header-card-text ">
                    <h2 id="take-your-ghost-blog-and-newsletter-to-the-next-level" class="kg-header-card-heading" style="color: #FFFFFF;" data-text-color="#FFFFFF"><span>Take your Ghost blog and newsletter to the next level.</span></h2>
                    <h3 id="let-our-team-handle-the-technology-while-you-focus-on-creating" class="kg-header-card-subheading" style="color: #FFFFFF;" data-text-color="#FFFFFF"><span>Let our team handle the technology while you focus on creating.</span></h3>
                    <a href="https://www.spectralwebservices.com/contact/" class="kg-header-card-button " style="background-color: #ffffff;color: #000000;" data-button-color="#ffffff" data-button-text-color="#000000">Contact Us</a>
                </div>
            </div>

Because there's no srcset on image, it loads at 2000px wide. That's silly for anything less than a 4k screen, and really silly for the 1200px wide screen I prefer. My browser actually rendered it at ~500px.

Ghost Version

5.59.1

Node.js Version

Ghost Pro and 16.something

How did you install Ghost?

ghost-cli and ghost pro

Database type

MySQL 8

Browser & OS version

Chrome, recent, windows 10

Relevant log / error output

No response

Code of Conduct

  • I agree to be friendly and polite to people in this repository
@github-actions github-actions bot added the needs:triage [triage] this needs to be triaged by the Ghost team label Aug 17, 2023
@witherScript
Copy link

witherScript commented Aug 18, 2023

@cathysarisky stoked to work on this issue, can you please assign it to me? If there are any specific guidelines or tasks you'd like me to follow outside of the contributions doc, please let me know.

@cathysarisky
Copy link
Contributor Author

cathysarisky commented Aug 18, 2023 via email

@daniellockyer daniellockyer added the bug [triage] something behaving unexpectedly label Aug 22, 2023
@github-actions github-actions bot removed the needs:triage [triage] this needs to be triaged by the Ghost team label Aug 22, 2023
@witherScript
Copy link

@daniellockyer hey, would be stoked to work on this issue, can you please assign it to me? I don't intend to muddy the issue thread

@cathysarisky
Copy link
Contributor Author

I'll add onto this one instead of opening another bug - these header images are also missing the lazy loading attribute that other body content gets automagically.

@kevinansfield kevinansfield added affects:editor Work relating to the Koenig Editor p2:major [triage] Bugs which we'll fix soon labels Nov 6, 2023
9larsons added a commit to TryGhost/Koenig that referenced this issue Nov 9, 2023
refs TryGhost/Ghost#17753
- added lazy loading attribute to header card renderer (v2+)
9larsons added a commit to TryGhost/Koenig that referenced this issue Nov 9, 2023
refs TryGhost/Ghost#17753
- added lazy loading attribute to header card renderer (v2+)
@kevinansfield kevinansfield self-assigned this Nov 13, 2023
kevinansfield added a commit to kevinansfield/Koenig that referenced this issue Nov 14, 2023
closes TryGhost/Ghost#17753

- added background image width and height attributes to `HeaderNode`
- updated header node components to capture width and height when an image is uploaded
- updated header node renderer to output `srcset` attribute on the `<picture>` element
kevinansfield added a commit to TryGhost/Koenig that referenced this issue Nov 14, 2023
closes TryGhost/Ghost#17753

- added background image width and height attributes to `HeaderNode`
- updated header node components to capture width and height when an image is uploaded
- updated header node renderer to output `srcset` attribute on the `<picture>` element
kevinansfield added a commit that referenced this issue Nov 14, 2023
closes TryGhost/Product#4146
closes #17753
closes TryGhost/Product#4127
closes #18903

- 🐛 Fixed blank render output in some cases when using line breaks
- 🐛 Fixed backspace at end of link sometimes deleting whole link in Firefox
- 🐛 Fixed plain black generated video thumbnails in Safari
- 🎨 Added `srcset` and `loading="lazy"` to header card images
- 🎨 Improved accessibility of buttons in render output by adding `aria-role` attributes
- 🎨 Removed Ctrl/Cmd+H shortcut as it clashed with expected OS shortcut
allouis pushed a commit that referenced this issue Nov 15, 2023
closes TryGhost/Product#4146
closes #17753
closes TryGhost/Product#4127
closes #18903

- 🐛 Fixed blank render output in some cases when using line breaks
- 🐛 Fixed backspace at end of link sometimes deleting whole link in Firefox
- 🐛 Fixed plain black generated video thumbnails in Safari
- 🎨 Added `srcset` and `loading="lazy"` to header card images
- 🎨 Improved accessibility of buttons in render output by adding `aria-role` attributes
- 🎨 Removed Ctrl/Cmd+H shortcut as it clashed with expected OS shortcut
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:editor Work relating to the Koenig Editor bug [triage] something behaving unexpectedly p2:major [triage] Bugs which we'll fix soon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants