Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Add the bird image to the homepage header #183

Merged
merged 9 commits into from
Nov 2, 2021

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Oct 29, 2021

Fixes #92.

This PR uses the new pattern block to place the header-large-dark pattern in the homepage's header. Like #182, this PR relies on WordPress/gutenberg#36090 to get it to appear correctly.

Before After
Screen Shot 2021-10-29 at 4 16 35 PM Screen Shot 2021-10-29 at 4 15 59 PM

@kjellr kjellr requested a review from jffng October 29, 2021 20:19
@kjellr kjellr self-assigned this Oct 29, 2021
@kjellr kjellr changed the title Add the bird images to the homepage header Add the bird image to the homepage header Oct 29, 2021
Copy link
Collaborator

@jffng jffng left a comment

Choose a reason for hiding this comment

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

Nice, love that we can remove repetitive code. Assuming the Gutenberg PR lands, this LGTM.

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 1, 2021

@jffng just pushed a quick update to re-use the header.html template part (as per the method in #55, which I think we still need to use). Could use another review just to be sure. 👍

@carolinan
Copy link
Collaborator

carolinan commented Nov 1, 2021

To get the dark header, I selected the header template part and used replace.
The image of the bird is showing, but the dark header is not the same width as earlier. Only the last, the inner most group block, is set to full width. 🤔
Ah, that's why the GB pr was needed.

@carolinan
Copy link
Collaborator

carolinan commented Nov 1, 2021

Also, the preview is not working for the dark header. which needs to be an upstream issue, if one hasn't already been opened

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 1, 2021

Also, the preview is not working for the dark header. which needs to be an upstream issue, if one hasn't already been opened

What are you seeing there? It did work for me, but it took a minute for the top of the header to appear:

Screen Shot 2021-11-01 at 9 30 26 AM

@carolinan
Copy link
Collaborator

I mean this preview not the pattern preview in the sidebar:
image

@carolinan
Copy link
Collaborator

Who has time to wait a full minute? :) (waiting doesn't help for this preview...)

@carolinan
Copy link
Collaborator

The problem with the theme attribute is that if I move the theme into a subfolder like wp-content/themes/theme-experiments/, this message is shown:
Template part has been deleted or is unavailable: header

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 2, 2021

I mean this preview not the pattern preview in the sidebar:

I'm already seeing errors with this preview in trunk, so I'm not sure it should hold up this PR:

Screen Shot 2021-11-02 at 9 51 36 AM

The problem with the theme attribute is that if I move the theme into a subfolder like wp-content/themes/theme-experiments/, this message is shown:
Template part has been deleted or is unavailable: header

I moved the theme into a subfolder and things continued to work as expected.

Even so, I tried out an alternate approach that uses a pattern just for the text + bird image, but it requires more code and also showed preview errors. So I'm not totally sure it's worth it.

Would someone mind testing this out in a subfolder to see if it works or not? If it's still a bug, I can push that alternate approach for now.

@jffng
Copy link
Collaborator

jffng commented Nov 2, 2021

This is what I see when I insert the dark header and the theme is a subfolder:

Screen Shot 2021-11-02 at 1 06 33 PM

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 2, 2021

Thanks for testing. I wonder why it worked for me. 🤔

Anyway, I pushed that second, more elaborate method. Would you mind giving that a try next?

@jffng
Copy link
Collaborator

jffng commented Nov 2, 2021

Anyway, I pushed that second, more elaborate method. Would you mind giving that a try next?

This worked for me with the theme in a subfolder, no template part not found error.

Copy link
Collaborator

@jffng jffng left a comment

Choose a reason for hiding this comment

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

Two small typo fixes and this LGTM.

Co-authored-by: Jeff Ong <jonger4@gmail.com>
Co-authored-by: Jeff Ong <jonger4@gmail.com>
@kjellr kjellr merged commit c892675 into trunk Nov 2, 2021
@kjellr kjellr deleted the update/use-template-part-in-header-dark branch November 2, 2021 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include the flying bird image in the homepage header
3 participants