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

Decode HTML Entities by default #50

Closed
sdegetaus opened this issue May 21, 2023 · 2 comments · Fixed by #54
Closed

Decode HTML Entities by default #50

sdegetaus opened this issue May 21, 2023 · 2 comments · Fixed by #54
Labels
status: confirmed 📍 The issue has been confirmed and reproduced. type: enhancement ⚡ Improves on an existing feature workaround provided 🩹 A temporary workaround has been provided.
Milestone

Comments

@sdegetaus
Copy link

What problem does this address?

When getting the values from the GraphQL, the titles and descriptions with HTML entities (such as – and &) are rendered as – and & respectively.

I think the plugin should display them ready-to-use by the frontend instead of the backend or frontend decoding them.

What is your proposed solution?

The obvious solution would be to decode them when passing them to the GraphQL.

What alternatives have you considered?

An alternative, is to do the decoding myself with RankMath's filters. For example, to decode the SEO title you can simply add these lines of code:

add_filter('rank_math/frontend/title', function ($title) {
  return html_entity_decode($title);
});

Nonetheless, I would still prefer that the plugin did this by default.
Let me know if you disagree and why 🙂

Additional Context

No response

@justlevine
Copy link
Member

Hey @sdegetaus - this is a great idea!

I'm not sure if it's safe to apply by default across the board ( ref: wp-graphql/wp-graphql#1035 ), but mosy are likely fine.

Since as you noted this does currently have a workaround, I'm going to slate this for a future improvement after we hit v0.1.0, but I'm happy to review any PRs if you (or anyone else) want to beat me to it.

PS: this issue is eligible for my WPGraphQL Spring Cleaning campaign, where I'm donating dev hours for for issues/PRs to the WPGraphQL project/feature of your choice. If you're interested, fill out the link above and let me know what I should work on.

@justlevine justlevine added workaround provided 🩹 A temporary workaround has been provided. status: confirmed 📍 The issue has been confirmed and reproduced. type: enhancement ⚡ Improves on an existing feature labels May 22, 2023
@justlevine justlevine added this to the Future milestone May 22, 2023
@justlevine
Copy link
Member

Hey @sdegetaus , #54 passes the SEO title, description and breadcrumbTitle through html_entities_decode().

If there's other fields that you think should be decoded as well, comment here and I'll create a new PR to address them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed 📍 The issue has been confirmed and reproduced. type: enhancement ⚡ Improves on an existing feature workaround provided 🩹 A temporary workaround has been provided.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants