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

Implement front page #164

Merged
merged 21 commits into from Jan 19, 2023
Merged

Implement front page #164

merged 21 commits into from Jan 19, 2023

Conversation

StevenDufresne
Copy link
Contributor

@StevenDufresne StevenDufresne commented Jan 13, 2023

Closes: #158

This PR implements the front page.

Questions:

  • The designs have links without text-decoration: underline;, but the new documentation site has them underlined. I assumed we want to make them the same. @javierarce @jasmussen
  • I'm not certain where all the links should go to @javierarce @jasmussen

Screenshot

Mobile Tablet Desktop
localhost_8888_(iPhone 12 Pro) (2) localhost_8888_ (1) localhost_8888_ (2)

Test

  1. Add Post Content below to the front page content.
  2. Replace broken URL in front-page-content.php with https://user-images.githubusercontent.com/1657336/212520185-d9840a21-b149-49c6-b4f2-c043eaf969ea.svg.
  3. Visit homepage.

Image For Testing

Link: https://user-images.githubusercontent.com/1657336/212520185-d9840a21-b149-49c6-b4f2-c043eaf969ea.svg

developers

@StevenDufresne StevenDufresne marked this pull request as ready for review January 14, 2023 12:24
@adamwoodnz

This comment was marked as resolved.

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Jan 15, 2023

Coding Standard

This is 'Coding Standards' in the design

@adamwoodnz

This comment was marked as resolved.

@StevenDufresne
Copy link
Contributor Author

The design shows the Search box aligning with 'Documentation' below

Ah okay. Broke it by adding edge-spacing for mobile. Good catch.

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

Looking good! A few minor suggestions inline

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Jan 16, 2023

One last thing, I received design feedback for Enterprise that the padding was too tight on sections like this on mobile (it should be 40px vertically)

So I ended up overriding a preset just for that section:

https://github.com/WordPress/wporg-main-2022/blob/trunk/source/wp-content/themes/wporg-main-2022/src/style/style.scss#L91

do_shortcode( 'wordpress_version_link' ),
do_shortcode( 'wordpress_version') ); ?>
</p>
<p class="has-text-align-center has-extra-small-font-size"><?php echo wp_kses_post( 'See <a href="[wordpress_version_link]">what has changed</a> in the WordPress [wordpress_version] API.' ); ?></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

N00b question; will this still show up as a translatable string? Or is that irrelevant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a noob question. I'm working too fast trying to get this project moving :).

This is missing translation. Thanks for the catch.

@StevenDufresne
Copy link
Contributor Author

One last thing, I received design feedback for Enterprise that the padding was too tight on sections like this on mobile (it should be 40px vertically)

That's a nice addition. I'll make the same change here. We can think more deeply about this should it keep coming up.

@jasmussen
Copy link
Collaborator

The designs have links without text-decoration: underline;, but the new documentation site has them underlined. I assumed we want to make them the same.

Consistency is good, but which non-underlined designs are you referring to? I haven't been as involved here, so forgive me if I'm missing something obvious. But most links seem underlined in the Figma.

Except those in the TOC, which should probably stay non-underlined:

Screenshot 2023-01-16 at 09 53 41

I'm not certain where all the links should go to
Advanced Administration?

@estelaris is this a question you can answer?

@ryelle
Copy link
Contributor

ryelle commented Jan 16, 2023

Out of curiosity, why did you decide to create a page template, pattern, template part, and have post content?

In testing, I noticed the hover & focus color for the white outlined button on blue is very low-contrast. This does match the design library, so this is probably a question for @jasmussen - is this correct?
Screenshot 2023-01-16 at 5 29 26 PM

The front page is missing an h1, the first heading on the page is the first section's title.

Screenshot 2023-01-16 at 5 44 22 PM

I'm not certain where all the links should go to
Advanced Administration?

This doesn't exist yet, as I understand it's a set of articles being moved from /support. According to this repo, it will eventually live at https://developer.wordpress.org/advanced-administration/.

HTTP API?

Maybe https://developer.wordpress.org/apis/making-http-requests/? Though that's a guess.

Contribute?

Probably make.wordpress.org.

Contributor Handbook?

Currently the site links to the documentation contributor handbook, but in this context it seems like it should be something more generic. There's this handbook, which is in progress but doesn't seem very helpful at the moment (for getting involved).

@StevenDufresne
Copy link
Contributor Author

The designs have links without text-decoration: underline;, but the new documentation site has them underlined. I assumed we want to make them the same.

Consistency is good, but which non-underlined designs are you referring to? I haven't been as involved here, so forgive me if I'm missing something obvious. But most links seem underlined in [the Figma](https://www.figma.com/file/iM8I9yAYbwjbc9q9b2WWui/Documentation---HelpHub-%2F-WordPress.org?node-id=2766%3A13537&

Developer Documentation
Figma Screen Shot 2023-01-17 at 9 15 40 AM Figma Screen Shot 2023-01-17 at 9 19 53 AM

@StevenDufresne
Copy link
Contributor Author

Out of curiosity, why did you decide to create a page template, pattern, template part, and have post content?

template part

The navigation link list was different in the designs from the main one. That was corrected after the fact. I could remove the template and use CSS to change its color.

pattern

Translations.

post content

I assumed all content had to be trackable in source control so I left only the link lists in the post content so it can be modified later.

@jasmussen
Copy link
Collaborator

In testing, I noticed the hover & focus color for the white outlined button on blue is very low-contrast. This does match the design library, so this is probably a question for @jasmussen - is this correct?

That doesn't look right, and won't fly. I don't think we necessarily need a hover style in this case (on this background) and the focus style can be white, a la what's on news:
Screenshot 2023-01-17 at 10 35 51

Screenshot 2023-01-17 at 10 36 00

@ryelle
Copy link
Contributor

ryelle commented Jan 17, 2023

That doesn't look right, and won't fly. I don't think we necessarily need a hover style in this case (on this background) and the focus style can be white, a la what's on news

Okay, we'll fix this in the parent theme for white-outline buttons on blueberry 👍🏻

@ryelle
Copy link
Contributor

ryelle commented Jan 17, 2023

The navigation link list was different in the designs from the main one. That was corrected after the fact. I could remove the template and use CSS to change its color.

Why not just add this code in the page template (templates/front-page.html) itself?

Translations.
I assumed all content had to be trackable in source control so I left only the link lists in the post content so it can be modified later.

Translations is a good reason for patterns, if there are plans to translate the site, but mixing the two (content & patterns) was what threw me off.

@StevenDufresne
Copy link
Contributor Author

The navigation link list was different in the designs from the main one. That was corrected after the fact. I could remove the template and use CSS to change its color.

Why not just add this code in the page template (templates/front-page.html) itself?

Translations.
I assumed all content had to be trackable in source control so I left only the link lists in the post content so it can be modified later.

Translations is a good reason for patterns, if there are plans to translate the site, but mixing the two (content & patterns) was what threw me off.

Fair, I think I'll always go with a pattern until told otherwise.

<!-- /wp:heading --></div>
<!-- /wp:group -->

<!-- wp:navigation {"textColor":"white","backgroundColor":"charcoal-2","className":"is-style-dropdown-on-mobile","style":{"spacing":{"blockGap":"24px"}},"fontSize":"small"} -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept this second navigation around for 2 reasons:

  • Making the colors match is possible with CSS, but all the styles have important and we lose the ability to easily customize it in the editor.
  • The site-breadcrumb doesn't match the designs. It would require extra CSS to make that work.

Having 2 link lists is not super dry, but the best of 2 evils in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could try __unstableLocation to reuse menu items in different navigation blocks by the location name, but it's unstable and requires a little setup in the local environment to make it work. I keep meaning to do an experiment branch to wporg-parent when we're between projects but… 😩

@StevenDufresne
Copy link
Contributor Author

I'm going to merge this for now and follow up with anything in the future.

@StevenDufresne StevenDufresne merged commit 5db6302 into trunk Jan 19, 2023
@StevenDufresne StevenDufresne deleted the try/front-page branch January 19, 2023 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layout: Front Page
4 participants