WordPress / community-themes

A collection of Block Themes built by the WordPress community.
80 stars 33 forks source link

Added 404 template #45

Closed kimclowa8c closed 1 year ago

kimclowa8c commented 1 year ago

Changes done in this PR

Mentions #12

carolinan commented 1 year ago

Thank you for working on this.

We would need to manually enqueue the stylesheet for both the editor and front. Let's try adding it in theme.json instead?

I did not fully understand why the padding needed to be on the input field and not the block.

kimclowa8c commented 1 year ago

Hey @carolinan Thanks for the feedback.

Your minor feedback here has the box checked. Just confirming, did you make this change?

We would need to manually enqueue the stylesheet for both the editor and front. Let's try adding it in theme.json instead?

I'm afraid I'm not a developer - I work mainly in the site editor and in blocks. Can you assist with this?

I did not fully understand why the padding needed to be on the input field and not the block.

The padding is for the word "search" inside of the input field. It's not for the block itself.

I would recommend having the same style for the block, no matter which template it is used in? I meant, I think we should remove .error404?

This was done purposefully in case different formatting is required for other search blocks throughout the site, perhaps on different background colours. However, happy to remove the block specification if you'd prefer to.

The height here is interesting because even with https://github.com/WordPress/community-themes/pull/41/files, we would not be able to add the height to the input field without custom CSS.

Yeah, the height in Figma was specified, but there isn't an option to set the height in the site editor, so I used CSS.

As a follow up we should either remove the pattern or move this content to the pattern.

Okay - I'm not familiar with this process but will check it out!

carolinan commented 1 year ago

I can make the code changes, but I wanted to discuss them first.