search

processing / p5.js-website

New p5.js website!
http://p5js.org
MIT License
19 stars 89 forks source link

Some symbols in code blocks in the reference (the description, not the examples) are msitakenly HTML-escaped #447

Closed davepagurek closed 2 months ago

davepagurek commented 4 months ago

Most appropriate sections of the p5.js website?

Reference

What is your operating system?

Mac OS

Web browser and version

Firefox

Actual Behavior

On https://p5js.org/reference/p5/mousePressed/, it contains this code snippet:

if (mouseX < 50) {
  // Code to run if the mouse is on the left.
}

Expected Behavior

if (mouseX < 50) {
  // Code to run if the mouse is on the left.
}

Steps to reproduce

Go to https://p5js.org/reference/p5/mousePressed/

Would you like to work on the issue?

Need to discuss the fix first

Abhinavcode13 commented 4 months ago 
if (mouseX &lt; 50) {
  // Code to run if the mouse is on the left.
}

don't this work fine ?

davepagurek commented 4 months ago

When users write code in the p5 web editor, they should be writing <, not &lt;, so the &lt; should not end up visible to users in the docs. Behind the scenes for this website it may still need to be represented like that, but we should make sure it gets rendered as the HTML character it represents instead.

Abhinavcode13 commented 4 months ago

so the most feasible fix should be to replace the &lt with < ?

davepagurek commented 4 months ago

It starts as < in the source of the documentation here:

https://github.com/processing/p5.js/blob/db01247bd224b71fbf253a546f6a35e65b988267/src/events/mouse.js#L1101-L1104

It's also still a < after it gets imported into the website repo, so the import script isn't the problem: https://github.com/processing/p5.js-website/blob/798342779820ea53d671cf85bf6b5731fb6f3c23/src/content/reference/en/p5/mousePressed.mdx?plain=1#L28-L30

It looks like we're intentionally escaping the contents of <code> blocks here: https://github.com/processing/p5.js-website/blob/798342779820ea53d671cf85bf6b5731fb6f3c23/src/pages/_utils.ts#L230-L244

Looking at the git history, this was to address https://github.com/processing/p5.js-website/issues/95, which is for inline code. I think what we want is to have this apply just to block code. So that would mean escaping <code> tag contents, but not the contents of <pre><code> I think? And we would need to do that in the escape function I referenced above.

Qianqianye commented 2 months ago

We noticed more issues about symbols and characters (like "<>") render incorrectly on the reference page, as mentioned in issue #522 #538, and PR #525 . Let's consolidate our discussion on how to fixes this in the issue.

Pasting comments from @limzykenneth in PR#525

For rendering in the page I'm not sure if Astro or the current build automatically encodes the characters so we need to check there first. For URL, adding a step to encode the URL path may be enough to solve this without needing to change the reference in the main p5.js repo.

Would love to hear the contributors involved in this to share their thoughts @limzykenneth @davepagurek @nickmcintyre @Abhinavcode13 @goldwasser @meganmckissack @isZhiyangWang. Thanks!