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 global header/footer design for News site #6

Closed
8 tasks done
iandunn opened this issue Aug 12, 2021 · 23 comments
Closed
8 tasks done

Implement global header/footer design for News site #6

iandunn opened this issue Aug 12, 2021 · 23 comments
Assignees
Labels
[Type] Enhancement New feature or request

Comments

@iandunn
Copy link
Member

iandunn commented Aug 12, 2021

https://www.figma.com/file/O9I8Jl7kaUMTX39LlRGbGO/News-Page?node-id=4659%3A8203

It may be easiest to get it working for News first, and the roll it out to the rest of the network in #7.

It should take into account any foreseeable needs for the network-wide rollout, though.

  • basic styles - mobile
  • basic styles - desktop
  • header menu
  • header search
  • overflow menu
  • responsive breakpoints
  • footer - mobile
  • footer - desktop
@iandunn
Copy link
Member Author

iandunn commented Aug 19, 2021

I was thinking of creating a "global header" dynamic block, and include that in block-template-parts/header.html (see #4 (comment)).

I don't think that's actually necessary, though. That block would contain the entire header area, and I'm assuming we're not going to want users changing the global header/footer.

So, I think we can just skip FSE for the header/footer area, and treat it like a classic theme. The sidebar/content area would still be FSE, though.

How does that sound @coreymckrill ?

@coreymckrill
Copy link
Contributor

If we can get that to work, that's probably fine for now. I still don't have a great understanding at this point of how block templates are loaded, and if you can even mix and match php and html template parts...

@iandunn
Copy link
Member Author

iandunn commented Aug 19, 2021

Oh, huh, it looks like the themes/wporg-news-2021/header.php is never used; maybe Blockbase included it as a fallback or something?

If it's not used, then I think we'll need a header block after all. I'll take a closer look tomorrow.

@coreymckrill
Copy link
Contributor

I think it's only used in the 404.php template.

@iandunn
Copy link
Member Author

iandunn commented Aug 19, 2021

huh, but there's a 404.html too.

maybe it's just for translation? Automattic/themes@ae7e63d

@coreymckrill
Copy link
Contributor

Ah, interesting. Good find.

@coreymckrill
Copy link
Contributor

I was thinking of creating a "global header" dynamic block, and include that in block-template-parts/header.html

Thinking more about this. It does seem like this would be feasible, for both the header and the footer, it could just use a server side render and not have any options or UI. Maybe it would go in a separate plugin, wporg-site-blocks or something?

I'm assuming we're not going to want users changing the global header/footer.

Yeah this seems related to #14. I haven't looked into whether specific templates can be locked (i.e. prevent users from editing them/creating versions stored in the db), but it seems like this should be doable. I suspect at first we'd just want to lock everything down, and then unlock things as the need arises.

@iandunn
Copy link
Member Author

iandunn commented Aug 20, 2021

@coreymckrill
Copy link
Contributor

Another thought: given the current limitations of FSE, we could have this theme take a hybrid approach: Use a theme.json file, but still use classic PHP-style templates instead of block templates. If we wanted to keep the door open for refactoring to full FSE later, the PHP templates could still include block grammar where feasible, so they'd be easier to convert to block templates in the future...

@iandunn
Copy link
Member Author

iandunn commented Aug 23, 2021

🤔 , my gut feeling is that'd introduce more technical debt overall, and it'd be better to commit to one or the other.

But then, if we end up having to create faux blocks for a lot of stuff... it might be less awkward to just use the parts of FSE that work for us today, and hold off on the others 🤷🏻‍♂️

I don't feel too strongly. Are you leaning one way or the other?

@coreymckrill
Copy link
Contributor

I've been vacillating on this. If it turns out that global header and footer are the only things we need "faux" blocks for, maybe we can, in fact, stick with block templates as well. I'm working on #17 right now, and I might have a firm opinion after that.

@iandunn
Copy link
Member Author

iandunn commented Aug 27, 2021

The foundation for this has been merged in WordPress/wporg-mu-plugins#4 and #20.

I'll work on the actual content next.

@iandunn
Copy link
Member Author

iandunn commented Aug 30, 2021

@coreymckrill, what was your process for creating the markup in #21?

I was thinking about using wp-admin/post-new.php?post_type=wp_template_part, and then copy/pasting the source into the PHP file. That seems like an awful devex, though, especially when iterating.

Did you come up with something better?

If not, maybe we could write an mu-plugin that would save wp-admin changes to the PHP file in addition to the db? That'd probably wipe out any PHP comments that we'd want, so we'd have to make them HTML comments, which would be a pain Gutenberg. I guess we could have a delimiter in the PHP file, and only things after it would be overwritten.

@iandunn
Copy link
Member Author

iandunn commented Aug 30, 2021

hey @beafialho 👋🏻
did you intend for the footer mockup to have this in the 6th column of links?

@WordPress
Wordpress [sic]

The current footer has those links, along w/ the Twitter/FB logos to their left. In Figma, the logos are floating in the bottom right, so they don't seem attached to the text, and the link text isn't descriptive enough for users to know what they'd be clicking on.

Should the text be removed in the new design, or did you want to keep it there and just have the logos in a different location?

@beafialho
Copy link
Collaborator

Hi @iandunn thanks for pointing that out, I missed that detail!

The intent is to have the social links be the icons at the bottom, so let's remove the last column of links.

footer

@iandunn
Copy link
Member Author

iandunn commented Aug 31, 2021

Sounds good, thanks!

iandunn added a commit that referenced this issue Aug 31, 2021
…ontent`

This will be necessary so that CSS and other assets will have a publicly accessible URL, so that they can be enqueued.

See WordPress/wporg-mu-plugins@422e41f
See #6
iandunn added a commit that referenced this issue Sep 1, 2021
These styles are artifacts of forking BlockBase, and the use of `!important` interferes with the global header/footer.

See #6
See #25
iandunn added a commit that referenced this issue Sep 1, 2021
Because they only contain a single element, they're no longer useful. Having them as a template part adds extra & unneccessary `<header>` / `<footer>` tags to the page.

See #6
iandunn added a commit to WordPress/wporg-mu-plugins that referenced this issue Sep 1, 2021
The styles will be refined in future commits.

See WordPress/wporg-news-2021#6
@iandunn
Copy link
Member Author

iandunn commented Sep 1, 2021

I took the liberty of adding Mobile and Patterns to the footer, since they're in the header. It doesn't make sense for anything in the header to be missing from the footer IMO.

I also added OpenVerse to the header and footer, since it should launch before this does. We can adjust if that changes though.

iandunn added a commit to WordPress/wporg-mu-plugins that referenced this issue Sep 1, 2021
* Rename files, since they'll no longer be directly `include`'d by classic themes
* Restore FSE editor formatting, to avoid future diff noise
* Add OpenVerse, since it'll launch before this does

See WordPress/wporg-news-2021#6
@beafialho
Copy link
Collaborator

Good idea, I see no reason why they shouldn't be there.

@coreymckrill
Copy link
Contributor

@iandunn

what was your process for creating the markup in #21?

I was thinking about using wp-admin/post-new.php?post_type=wp_template_part, and then copy/pasting the source into the PHP file. That seems like an awful devex, though, especially when iterating.

Did you come up with something better?

Nope. I used the block editor a few times and just switched to Code View to copy/paste, but after I had enough sample material in my files, I just started copy/pasting it from there and changing the block names and attributes by hand.

@iandunn
Copy link
Member Author

iandunn commented Sep 7, 2021

Yeah, that's what I've mostly ended up doing too 👍🏻

@iandunn
Copy link
Member Author

iandunn commented Sep 8, 2021

@beafialho , in the mockup, the header menu is 1320px wide; how should it look at a normal desktop size, like the 900-1100px range?

Should the menu wrap to a 2nd row? If so, what kind of spacing do you want, etc?

@beafialho
Copy link
Collaborator

@iandunn this should be the behaviour of the header in different screen sizes:

  • In the largest screen size, the menu links should stick to the left
  • The first element to be hidden should always be the logo (leaving the WP icon only), followed by the "Get WordPress" button
  • The menu should collapse at 876px turning into the mobile option

I've added a responsive section to the Figma file with all these mockups there.

header

iandunn added a commit that referenced this issue Sep 22, 2021
iandunn added a commit that referenced this issue Sep 22, 2021
iandunn added a commit that referenced this issue Sep 22, 2021
iandunn added a commit that referenced this issue Sep 22, 2021
iandunn added a commit that referenced this issue Sep 22, 2021
iandunn added a commit to WordPress/wporg-mu-plugins that referenced this issue Sep 24, 2021
iandunn added a commit that referenced this issue Sep 24, 2021
iandunn added a commit to WordPress/wporg-mu-plugins that referenced this issue Sep 27, 2021
iandunn added a commit to WordPress/wporg-mu-plugins that referenced this issue Sep 27, 2021
* Header/Footer: Refine header styles.

See WordPress/wporg-news-2021#6
iandunn added a commit that referenced this issue Sep 29, 2021
iandunn added a commit that referenced this issue Sep 29, 2021
@iandunn
Copy link
Member Author

iandunn commented Oct 15, 2021

This should be all done, except for pixel perfect tweaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants