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

Add title class name to latest posts Block #32374

Conversation

miminari
Copy link
Member

@miminari miminari commented Jun 1, 2021

Description

I think the title element of the latest posts Block should contain the class name, just like the author, post date, etc.

related #22759

How has this been tested?

  1. You create a new post.
  2. Add a latest post block, and save.

You will see the class name wp-block-latest-posts__post-title added to the a tag of the post title in the editor screen and on the front side using the dev tool.

Screenshots

 ss_2021-06-01 23 30 01

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@miminari miminari requested a review from ajitbohra as a code owner June 1, 2021 14:33
@miminari miminari added the [Block] Latest Posts Affects the Latest Posts Block label Jun 1, 2021
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @miminari, classes are not part of the public WordPress API and they can be changed or removed. Normally we only add classes when we have a use case for them (as part of the styling core/Gutenberg does) I guess we can add the class if one day we need to use it?

@miminari
Copy link
Member Author

miminari commented Jun 3, 2021

@jorgefilipecosta Thank you for your reply. OK, that is the policy of WordPress, but I don't think it explains why other elements are given classes and why only the title element doesn't need to be styled. Why not? It is uncomfortable. If you try to apply a style to this block in your theme, I'm sure you will feel the same discomfort. And since it's an a tag, you have to be a little careful when you try to give it a style according to "the stylelint". Conversely, why is the class name given to the time tag?

@aristath
Copy link
Member

aristath commented Jun 3, 2021

I'm afraid I don't quite understand why this would be necessary... That block doesn't have any text links in it other than the post-titles, so .wp-block-latest-posts a will work just fine without affecting anything else 🤔
Am I missing something?

@miminari
Copy link
Member Author

miminari commented Jun 6, 2021

@aristath Thank you for reply. What I'm trying to tell you is that the coding rule may be ambiguous. If the rule is to add classes only for the minimum required by the core, then, the class name of the time tag doesn't seem to be necessary, does it?

@dashkevych
Copy link

I'm afraid I don't quite understand why this would be necessary... That block doesn't have any text links in it other than the post-titles, so .wp-block-latest-posts a will work just fine without affecting anything else 🤔
Am I missing something?

The block allows to display the content of the post (either excerpt or full post). So, I believe having .wp-block-latest-posts a will also affect links inside the content area, including the Read More link.

@skorasaurus
Copy link
Member

Hi, thanks for your effort in making this. Since your PR was made, another PR was made and has been merged into Gutenberg that adds the title class name; fulfilling the purpose of your PR.

@miminari miminari deleted the try/class-name-latest-post-title branch May 3, 2022 12:01
@adrianahdez
Copy link

adrianahdez commented Apr 12, 2024

Hi @skorasaurus, I know that the PR you referenced is merged but I found that it is wrong, because the class for post title was added on the featured image element instead on the post title element. I've created a new issue for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Posts Affects the Latest Posts Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants