Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

#124 - Project Details Block Pattern #189

Merged

Conversation

khleomix
Copy link
Contributor

@khleomix khleomix commented Sep 4, 2023

Description

Screenshots
Desktop
image

Mobile
image

Testing Instructions

  1. Create/Edit a page.
  2. Add a pattern -> text -> Project Details
  3. Edit where necessary

Contributors
khleomix

@MaggieCabrera
Copy link
Collaborator

The markup for this pattern was already provided by Bea but since it wasn't on a template I didn't include it in the original PR. Can you please check your markup against hers here to check that it all aligns?

@khleomix
Copy link
Contributor Author

khleomix commented Sep 4, 2023

@MaggieCabrera I've updated the pattern to the provided markup. Let me know if that works.

Copy link
Contributor

@maneshtimilsina maneshtimilsina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few of my quick findings.

  • Please use the spacing presets instead of arbitrary values for paddings where needed.
  • Use esc_html_x with context instead of esc_html. You can check 404.php or other approved patterns for the reference.

?>
<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"8vh","bottom":"8vh","right":"6vw","left":"6vw"}}},"backgroundColor":"base","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull has-base-background-color has-background" style="padding-top:8vh;padding-right:6vw;padding-bottom:8vh;padding-left:6vw"><!-- wp:columns {"align":"wide","style":{"spacing":{"blockGap":{"top":"2em","left":"2em"}}}} -->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use presets instead here for the padding?

<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"8vh","bottom":"8vh","right":"6vw","left":"6vw"}}},"backgroundColor":"base","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull has-base-background-color has-background" style="padding-top:8vh;padding-right:6vw;padding-bottom:8vh;padding-left:6vw"><!-- wp:columns {"align":"wide","style":{"spacing":{"blockGap":{"top":"2em","left":"2em"}}}} -->
<div class="wp-block-columns alignwide"><!-- wp:column {"width":"15%"} -->
<div class="wp-block-column" style="flex-basis:15%"><!-- wp:paragraph {"style":{"typography":{"fontSize":"0.8rem","lineHeight":"1.4"}}} -->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same about font size presets here

Copy link
Collaborator

@MaggieCabrera MaggieCabrera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the spacing values, let's bring this in and iterate

@MaggieCabrera MaggieCabrera merged commit cc76181 into WordPress:trunk Sep 6, 2023
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants