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

Unclear meaning block. #482

Open
renintw opened this issue Jan 22, 2024 · 2 comments
Open

Unclear meaning block. #482

renintw opened this issue Jan 22, 2024 · 2 comments
Labels
Redesign [Type] Bug Something isn't working [Type] Question Needs input from others before actionable ui
Milestone

Comments

@renintw
Copy link
Contributor

renintw commented Jan 22, 2024

Found when reviewing #447.
There's a code block with unclear meaning.
Example: http://localhost:8895/reference/classes/wp_query/#standard-loop

image

@renintw renintw added [Type] Bug Something isn't working ui Redesign labels Jan 22, 2024
@renintw renintw added this to the Iteration 1 milestone Jan 22, 2024
@renintw renintw assigned renintw and unassigned renintw Jan 22, 2024
@renintw
Copy link
Contributor Author

renintw commented Jan 22, 2024

Some findings:

  1. It can be fixed by removing <pre class="notranslate"> in the shortcode.
  2. It also can be fixed by adding an extra <pre> tag around the %2$s here.
  3. Once there's <pre></pre> inside the shortcode, there'd be an extra <p> + <code> as shown in the PR description.

You can test with the following content in a new post.

<!-- wp:shortcode -->
[php]
<span class="pl-ent">&lt;?php</span>
<pre class="notranslate"><span class="pl-c">// The Query.</span>
<span class="pl-s1"><span class="pl-c1">$</span>the_query</span> = <span class="pl-k">new</span> <span class="pl-v">WP_Query</span>( <span class="pl-s1"><span class="pl-c1">$</span>args</span> );

<span class="pl-c">// The Loop.</span>
<span class="pl-k">if</span> ( <span class="pl-s1"><span class="pl-c1">$</span>the_query</span>-&gt;<span class="pl-en">have_posts</span>() ) {
	<span class="pl-k">echo</span> <span class="pl-s">'&lt;ul&gt;'</span>;
	<span class="pl-k">while</span> ( <span class="pl-s1"><span class="pl-c1">$</span>the_query</span>-&gt;<span class="pl-en">have_posts</span>() ) {
		<span class="pl-s1"><span class="pl-c1">$</span>the_query</span>-&gt;<span class="pl-en">the_post</span>();
		<span class="pl-k">echo</span> <span class="pl-s">'&lt;li&gt;'</span> . esc_html( get_the_title() ) . <span class="pl-s">'&lt;/li&gt;'</span>;
	}
	<span class="pl-k">echo</span> <span class="pl-s">'&lt;/ul&gt;'</span>;
} <span class="pl-k">else</span> {
	esc_html_e( <span class="pl-s">'Sorry, no posts matched your criteria.'</span> );
}
<span class="pl-c">// Restore original Post Data.</span>
wp_reset_postdata();</pre>
[/php]
<!-- /wp:shortcode -->

Before diving further into this:

  1. After the fix from Update inline code padding and use border-radius variable #447, the block will be invisible. However, it still occupies space. Visually, it looks like bottom padding. Ideally, it would be better to remove it, but if the method of removal is complicated or tricky, perhaps it's okay to leave it for now?
  2. Is anyone familiar with the reason here - why in the shortcode, the addition of the pre tag comes with an extra <p> tag. Is this intentional?

cc @ryelle @adamwoodnz

@renintw renintw added [Type] Question Needs input from others before actionable [Status] On Hold Work is paused temporarily labels Jan 29, 2024
@renintw renintw removed their assignment Jan 29, 2024
@ryelle
Copy link
Contributor

ryelle commented Jan 29, 2024

I think all the HTML in the shortcode is legacy or maybe carried over from copy-paste, since the syntax-highlighting is done by prism.js now. I imagine the parsing that prism.js causes the extra nested code.

Is this an issue on more than one page? Do we need a code solution, or could we just update the content in the explanation here?

@adamwoodnz adamwoodnz removed the [Status] On Hold Work is paused temporarily label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Redesign [Type] Bug Something isn't working [Type] Question Needs input from others before actionable ui
Projects
None yet
Development

No branches or pull requests

3 participants