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

Add About subpages #176

Merged
merged 11 commits into from
Feb 2, 2023
Merged

Add About subpages #176

merged 11 commits into from
Feb 2, 2023

Conversation

adamwoodnz
Copy link
Contributor

@adamwoodnz adamwoodnz commented Jan 23, 2023

Add redesigned subpages of w.org/about: Requirements, Features, Security, Roadmap, and History.

Closes #149

Screenshots

Requirements Features Security Roadmap History
localhost_8888_about-2_requirements-2_(Desktop) localhost_8888_about-2_features-2_(Desktop) localhost_8888_about-2_security-2_(Desktop) localhost_8888_about-2_roadmap-2_(Desktop) localhost_8888_about-2_history-2_(Desktop)

How to test the changes in this Pull Request:

  1. Add the following pages to your site:
'/about-2/',
'/about-2/requirements-2/',
'/about-2/features-2/',
'/about-2/security-2/',
'/about-2/roadmap-2/',
'/about-2/history-2/',
  1. Browse the pages using the navigation and internal links on '/about-2/security-2/',
  2. Check responsive layout
  3. Check escaping of text in patterns

@adamwoodnz adamwoodnz added [Component] Theme Templates, patterns, CSS [Component] Content Bugs or issues related to the page content labels Jan 23, 2023
@adamwoodnz adamwoodnz added this to the About milestone Jan 23, 2023
@adamwoodnz adamwoodnz self-assigned this Jan 23, 2023
@adamwoodnz adamwoodnz changed the base branch from trunk to add/148-about-page January 23, 2023 03:25
@adamwoodnz adamwoodnz marked this pull request as draft January 23, 2023 03:25
@adamwoodnz adamwoodnz force-pushed the add/149-about-subpages branch 5 times, most recently from 911037e to b76b4d0 Compare January 25, 2023 01:55

<!-- wp:group {"style":{"spacing":{"blockGap":"var:preset|spacing|10"}},"layout":{"type":"constrained"}} -->
<div id="footnotes" class="wp-block-group"><!-- wp:paragraph -->
<p id="footnote1"><?php _e( '<a href="#ref1">[1]</a>&nbsp;<a href="https://w3techs.com/">https://w3techs.com/</a>, as of December 2019', 'wporg' ); ?></p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original page had these as list items, but it isn't possible to add ids to list items at present. This is the closest I could get and still have the ref to footnote linking functional.

@adamwoodnz adamwoodnz force-pushed the add/149-about-subpages branch from ad066fe to b9280d6 Compare January 26, 2023 02:13
@adamwoodnz adamwoodnz force-pushed the add/149-about-subpages branch 2 times, most recently from 34429ef to bb936ba Compare January 26, 2023 04:28
@adamwoodnz adamwoodnz mentioned this pull request Jan 29, 2023
20 tasks
@adamwoodnz adamwoodnz force-pushed the add/149-about-subpages branch from bb936ba to f2e6667 Compare January 30, 2023 00:31
@adamwoodnz adamwoodnz marked this pull request as ready for review January 30, 2023 00:31
@ryelle
Copy link
Contributor

ryelle commented Jan 31, 2023

Should the local nav bar be sticky like it is on the Download subpages? for example: https://wordpress.org/download/releases/

@adamwoodnz
Copy link
Contributor Author

Should the local nav bar be sticky like it is on the Download subpages?

Probably, nice catch. I'll update.

@adamwoodnz adamwoodnz force-pushed the add/149-about-subpages branch 2 times, most recently from 056bf37 to 96b792d Compare February 1, 2023 04:00
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

Generally this is looking good, just the list spacing issue & the local nav mobile styles.

<p class="has-small-font-size" style="font-style:normal;font-weight:700;line-height:1.9"><?php _e( 'About', 'wporg' ); ?></p>
<!-- /wp:paragraph -->

<!-- wp:navigation {"ref":3,"textColor":"blueberry-1","align":"wide","layout":{"type":"flex","justifyContent":"right"},"fontSize":"small"} /--></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this nav ref is incorrect, the others are 16421.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it was fixed when I ran build:patterns 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops that's my local nav menu id, thanks 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

<!-- /wp:paragraph -->

<!-- wp:list -->
<ul><!-- wp:list-item -->
Copy link
Contributor

Choose a reason for hiding this comment

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

The list here should have space between the list items. Does the list block support a blockGap? Or maybe we need a block style for "long content lists" or something?

Mockup Page
Screenshot 2023-02-01 at 1 41 00 PM Screenshot 2023-02-01 at 1 40 45 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR opened as the list block doesn't support block gap: WordPress/wporg-parent-2021#72

Comment on lines 9 to 17
<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"var:preset|spacing|20","right":"var:preset|spacing|edge-space","bottom":"var:preset|spacing|20","left":"var:preset|spacing|edge-space"}}},"backgroundColor":"white","className":"is-sticky","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull is-sticky has-white-background-color has-background" style="padding-top:var(--wp--preset--spacing--20);padding-right:var(--wp--preset--spacing--edge-space);padding-bottom:var(--wp--preset--spacing--20);padding-left:var(--wp--preset--spacing--edge-space)"><!-- wp:group {"align":"full","layout":{"type":"flex","flexWrap":"nowrap","justifyContent":"space-between"}} -->
<div class="wp-block-group alignfull"><!-- wp:paragraph {"style":{"typography":{"fontStyle":"normal","fontWeight":"700","lineHeight":1.9}},"fontSize":"small"} -->
<p class="has-small-font-size" style="font-style:normal;font-weight:700;line-height:1.9"><?php _e( 'About', 'wporg' ); ?></p>
<!-- /wp:paragraph -->

<!-- wp:navigation {"ref":16421,"textColor":"blueberry-1","align":"wide","className":"is-style-default","layout":{"type":"flex","justifyContent":"right"},"fontSize":"small"} /--></div>
<!-- /wp:group --></div>
<!-- /wp:group -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"var:preset|spacing|20","right":"var:preset|spacing|edge-space","bottom":"var:preset|spacing|20","left":"var:preset|spacing|edge-space"}}},"backgroundColor":"white","className":"is-sticky","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull is-sticky has-white-background-color has-background" style="padding-top:var(--wp--preset--spacing--20);padding-right:var(--wp--preset--spacing--edge-space);padding-bottom:var(--wp--preset--spacing--20);padding-left:var(--wp--preset--spacing--edge-space)"><!-- wp:group {"align":"full","layout":{"type":"flex","flexWrap":"nowrap","justifyContent":"space-between"}} -->
<div class="wp-block-group alignfull"><!-- wp:paragraph {"style":{"typography":{"fontStyle":"normal","fontWeight":"700","lineHeight":1.9}},"fontSize":"small"} -->
<p class="has-small-font-size" style="font-style:normal;font-weight:700;line-height:1.9"><?php _e( 'About', 'wporg' ); ?></p>
<!-- /wp:paragraph -->
<!-- wp:navigation {"ref":16421,"textColor":"blueberry-1","align":"wide","className":"is-style-default","layout":{"type":"flex","justifyContent":"right"},"fontSize":"small"} /--></div>
<!-- /wp:group --></div>
<!-- /wp:group -->
<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"16px","right":"var:preset|spacing|60","bottom":"0","left":"var:preset|spacing|60"}}},"backgroundColor":"white","textColor":"blueberry-1","className":"is-sticky","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull is-sticky has-blueberry-1-color has-white-background-color has-text-color has-background" style="padding-top:16px;padding-right:var(--wp--preset--spacing--60);padding-bottom:0;padding-left:var(--wp--preset--spacing--60)"><!-- wp:group {"align":"full","layout":{"type":"flex","flexWrap":"wrap","justifyContent":"space-between"}} -->
<div class="wp-block-group alignfull"><!-- wp:paragraph {"style":{"typography":{"fontStyle":"normal","fontWeight":"700"}},"textColor":"charcoal-1","fontSize":"small"} -->
<p class="has-charcoal-1-color has-text-color has-small-font-size" style="font-style:normal;font-weight:700"><?php _e( 'About', 'wporg' ); ?></p>
<!-- /wp:paragraph -->
<!-- wp:navigation {"ref":16421,"textColor":"blueberry-1","className":"is-style-dropdown-on-mobile","style":{"spacing":{"blockGap":"24px"}},"fontSize":"small"} /--></div>
<!-- /wp:group --></div>
<!-- /wp:group -->

The banner needs a little tweaking for the mobile dropdown to work as a dropdown instead of a full overlay, like it does on the Download section. We have a special is-style-dropdown-on-mobile class for that. The group blocks layout also needed some updates.

I saved this ^^ change on the requirements page, but I didn't follow through with the rest of the pages, and the link color in the dropdown needs to be updated.

this suggestion current branch
Screen Shot 2023-02-01 at 13 55 28 Screen Shot 2023-02-01 at 14 05 33

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated all these. The link colors were fixed by adding a background color to the nav, thereby avoiding this Gutenberg style:

wp-block-navigation:not(.has-background) .wp-block-navigation__responsive-container.is-menu-open {
    color: #000;
}

Comment on lines +9 to +10
<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"right":"var:preset|spacing|60","left":"var:preset|spacing|60","top":"16px","bottom":"16px"}}},"backgroundColor":"white","className":"is-sticky","layout":{"type":"constrained"}} -->
<div class="wp-block-group alignfull is-sticky has-white-background-color has-background" style="padding-top:16px;padding-right:var(--wp--preset--spacing--60);padding-bottom:16px;padding-left:var(--wp--preset--spacing--60)"><!-- wp:group {"align":"full","layout":{"type":"flex","flexWrap":"wrap","justifyContent":"space-between"}} -->
Copy link
Contributor Author

@adamwoodnz adamwoodnz Feb 1, 2023

Choose a reason for hiding this comment

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

Added 16px bottom padding to this, in place of the brushstroke we have on the Download sub nav, keeps the spacing consistent when scrolled:

Screen Shot 2023-02-02 at 11 54 19 AM

Screen Shot 2023-02-02 at 12 04 57 PM

Base automatically changed from add/148-about-page to trunk February 2, 2023 01:45
@adamwoodnz adamwoodnz force-pushed the add/149-about-subpages branch from 5279a11 to a1a375c Compare February 2, 2023 02:30
@adamwoodnz adamwoodnz merged commit 17949df into trunk Feb 2, 2023
@adamwoodnz adamwoodnz deleted the add/149-about-subpages branch February 2, 2023 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Content Bugs or issues related to the page content [Component] Theme Templates, patterns, CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add About page content
2 participants