Skip to content
This repository has been archived by the owner on Dec 1, 2019. It is now read-only.

Fixed - Two <h1> on static frontpage #496

Merged
merged 8 commits into from Sep 24, 2019

Conversation

acosmin
Copy link
Contributor

@acosmin acosmin commented Sep 24, 2019

Fix for #490

@mukeshpanchal27
Copy link
Member

mukeshpanchal27 commented Sep 24, 2019

@acosmin Above fix remove h1 from Posts page.

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

@mukeshpanchal27 it looks like that, I'll look into it... the same issue can be found in twentynineteen

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

@mukeshpanchal27 please go ahead and try it :) I think I got it right now 😄 I think a patch for twentynineteen is also needed

@mukeshpanchal27
Copy link
Member

@acosmin latest patch working fine for me.

@nielslange @ntwb can you please review this.

inc/template-tags.php Outdated Show resolved Hide resolved
@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

I hope this one works :)

@nielslange
Copy link
Collaborator

nielslange commented Sep 24, 2019

try nr.2

@acosmin As many folks contributing to this repo it would be better to use speaking commit messages such as Enhance title conditions, Adjust <h1> conditions or something along these lines instead of try nr.2, which could be pretty much everything. 😁

Personally, I also add the issue ID to the commit message (if I do not apply changes directly to the origin) to highlight which issue is affected by the commit: https://github.com/WordPress/twentytwenty/commits?author=nielslange

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

Agreed, I'll do that from now on 😄

Also, I think this last one did fix it 🤣

@nielslange
Copy link
Collaborator

@acosmin I'm not sure if is_page_template() can replace is_archive() and is_author() though. Wanna give try nr.3 a try? 😂

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

@nielslange not really, I think this is the final one 😆 try it out, it works for me on archive and author pages... and the rest of them

@nielslange
Copy link
Collaborator

@acosmin Indeed, the latest commit fixed it. LGTM! 🙌

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

twentynineteen also has some of these problems

@nielslange
Copy link
Collaborator

twentynineteen also has some of these problems

You know how to fix it now, don't you? 😛

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

I do but I hate trac :)

@nielslange
Copy link
Collaborator

I know what you mean. I'm not a big fan of Trac either, but you get used to it. 😉

@pattonwebz
Copy link
Member

I made a ticket XD https://core.trac.wordpress.org/ticket/48126

Copy link
Contributor

@ianbelanger79 ianbelanger79 left a comment

Choose a reason for hiding this comment

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

@acosmin This does not seem to fix the issue when using a static homepage. I still see 2 <h1>'s

@nielslange
Copy link
Collaborator

nielslange commented Sep 24, 2019

@acosmin This does not seem to fix the issue when using a static homepage. I still see 2 <h1>'s

Are you using the theme unit test file, @ianbelanger79. If so, there's a high chance that the second <h1> is coming from the post Markup: HTML Tags and Formatting, which contains <h1> until <h6>. You wrote "static homepage". My bad! 🤦‍♂

@acosmin
Copy link
Contributor Author

acosmin commented Sep 24, 2019

@ianbelanger79 is_page() should take care of is_page_template() and other cases...

Copy link
Contributor

@ianbelanger79 ianbelanger79 left a comment

Choose a reason for hiding this comment

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

That fixed it

@ianbelanger79 ianbelanger79 merged commit 6306c74 into WordPress:master Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants