Lesson Plans: Landing page, archive and single page layouts

[danhthong]

trafficstars

This PR for #153

Steps to update Lesson Plans landing page.

• Update Dashicon & select categories to show on landing page https://a.cl.ly/kpu8xe5d
• Update Dashicon & select Audiences to show on landing page https://a.cl.ly/DOudYJ92

[hlashbrooke]

This landing page looks great! Excellent work and thanks for contributing @danhthong! I will need someone from the dev team to do a code review - perhaps @adamwoodnz? - but the layout works well from my side!

One note is that this will not be set as the home page of the site, but will need to live at /lesson-plans. That means we will need to ensure this page works at that URL while maintaining the filtered lesson plans archive page that the buttons on this landing page link to. I'm unsure how to make that work, but I'm sure it can be figured out.

[danhthong]

Page needs to exist at /lesson-plans while still maintaining the filtered lesson plan archive pages.

@hlashbrooke I have updated as your requested to keep current url by d8783ab.

[adamwoodnz]

This archive-title-prefix isn't in the Figma file.

I can see it's existing though. Do we want to keep it @courtneyr-dev @hlashbrooke ?

[adamwoodnz]

Bug: if I submit an 'Edit audience' or 'Edit Lesson Category' form with sticky unchecked I get an error page with

Notice: Undefined index: sticky in /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php on line 643 Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php:643) in /var/www/html/wp-includes/pluggable.php on line 1420 Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php:643) in /var/www/html/wp-includes/pluggable.php on line 1423

[danhthong]

Great job @danhthong!

There is a lot of feedback here but a lot of it is pretty minor; renaming and refactoring, etc. If you're open to it I could help out addressing some of these things?

Let me know!

Thank you so much @adamwoodnz

I will update and let you know once i updated all your feedback.

[adamwoodnz]

Nice updates @danhthong, thanks!

I didn't test in small screens last time and I'm seeing a few responsive issues now. Vertical spacing between sections, and where buttons drop below text needs attention on all small sizes. On the 460 > 767 range the cards don't have margin on the sides.

iPad	460 > 767	iPhone

On iPad some of the column layout isn't quite right; I think Level and Duration could still sit side-by-side

The Format grid of buttons should still be 4 column I think:

If you need design guidance on this maybe @melchoyce could assist? Otherwise I'm happy to make some adjustments to vertical spacing once the column and horizontal margins are sorted.

[azhiya]

@hlashbrooke apologies is there anything else you need from me.

[danhthong]

I didn't test in small screens last time and I'm seeing a few responsive issues now. Vertical spacing between sections, and where buttons drop below text needs attention on all small sizes. On the 460 > 767 range the cards don't have margin on the sides.

I have updated as commit 9848f6d

Iphone mini(375): https://a.cl.ly/yAuQPogP 460 > 767: https://a.cl.ly/YEuWj86Q Ipad: https://a.cl.ly/6quGR0Ar

[danhthong]

I haven't tested it locally, but the code looks good. I left a few comments, but most are minor.

@iandunn I have updated your requested as 37ef929 Thank for your suggested!

[adamwoodnz]

Bug: if I submit an 'Edit audience' or 'Edit Lesson Category' form with sticky unchecked I get an error page with

Notice: Undefined index: sticky in /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php on line 643 Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php:643) in /var/www/html/wp-includes/pluggable.php on line 1420 Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php:643) in /var/www/html/wp-includes/pluggable.php on line 1423

@danhthong I am still seeing this issue if 'Check to show on landing page' is left unchecked when saving, eg.

Notice: Undefined index: sticky in /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php on line 637 Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php:637) in /var/www/html/wp-includes/pluggable.php on line 1420 Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/wp-content/plugins/wporg-learn/inc/taxonomy.php:637) in /var/www/html/wp-includes/pluggable.php on line 1423

I guess $_POST['sticky'] is undefined?

update_term_meta(
		$term_id,
		'sticky',
		rest_sanitize_boolean( $_POST['sticky'] ) ?? 0
	);

[adamwoodnz]

Hi @danhthong last couple of things and then I'll be happy to deploy this, thanks!

[danhthong]

Hi @danhthong last couple of things and then I'll be happy to deploy this, thanks!

Thank @adamwoodnz , I have updated as above.

[adamwoodnz]

@danhthong @isvictorious this last commit was a little unexpected... was it intentional?

[isvictorious]

@danhthong @isvictorious this last commit was a little unexpected... was it intentional?

It was intentional but not where it was supposed to go. We can roll back.

[danhthong]

@danhthong @isvictorious this last commit was a little unexpected... was it intentional?

@adamwoodnz I rolled back to last commit.

[adamwoodnz]

Hi @danhthong this just needs a rebase now and then let's get it merged!

[danhthong]

Hi @danhthong this just needs a rebase now and then let's get it merged!

Hi @adamwoodnz, I have rebased and have a commit for css codes lint at 9d2524e7a. Please let me know if you need anything else.

[adamwoodnz]

Thanks @danhthong, seems we've got a bunch of new unexpected PHP lint issues relating to code in wporg-mu-plugins now that we've rebased. An existing exclude rule exists for mu-plugins/pub but it doesn't seem to be covering mu-plugins/wporg-mu-plugins:

<exclude-pattern>/wp-content/mu-plugins/pub/(?!locales.php)</exclude-pattern>

I think adding a new exclude like this will remove most:

<exclude-pattern>/wp-content/mu-plugins/wporg-mu-plugins</exclude-pattern>

The remaining fixes required should be close to this:

[adamwoodnz]

I'll try to get some of these fixed today so you can rebase on trunk again.

[adamwoodnz]

I have created https://github.com/WordPress/Learn/pull/934 to fix these failures. You may want to rebase on that unless it's been merged when you read this @danhthong

[adamwoodnz]

@danhthong the lint failures have now been fixed in trunk. You'll need to rebase again I'm afraid, but then we should be good to merge.

[danhthong]

@adamwoodnz All done. Thank you a lot!

[adamwoodnz]

I'm seeing some odd styling on the Learn homepage cards

They appear to be missing this padding rule

Same for you @danhthong ?

[hlashbrooke]

IIRC that padding will be off for you because it is actually coming from the Customiser on the live Learn site. I will confess that I added it (along with some other custom CSS) when I wasn't able to contribute code properly but needed the styling to be done.

Now that I think of it, I'll create a new issue for moving any CSS from the Customiser into the actual theme files.

UPDATE: Added an issue here: https://github.com/WordPress/Learn/issues/937

[adamwoodnz]

IIRC that padding will be off for you because it is actually coming from the Customiser on the live Learn site.

Adding this locally fixed it, thanks @hlashbrooke

I'll approve this now, and merge and deploy once I get the all clear from @iandunn.

We'll need to select the categories and audiences for the landing page cards and choose dashicons for them after deployment.

[hlashbrooke]

Thanks for all the hard work here @danhthong and @adamwoodnz - this is going be great to have live!

[danhthong]

@adamwoodnz I updated for last issue and add check to skip phpcs ignores.