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

[#69] Page Header Component #79

Merged
merged 6 commits into from
May 23, 2024
Merged

[#69] Page Header Component #79

merged 6 commits into from
May 23, 2024

Conversation

bd-viget
Copy link
Contributor

@bd-viget bd-viget commented May 20, 2024

Summary

This PR adds the Page Header Component. The basic styles have been added with plain CSS, so it's going to need some of @nathan-schmidt-viget's Tailwind love!

Issues

Testing Instructions

  1. Create a new page
  2. Add the Page Header block

Screenshots

Screenshot 2024-05-20 at 5 11 52 PM Screenshot 2024-05-20 at 5 15 20 PM

@bd-viget bd-viget self-assigned this May 20, 2024
Copy link
Contributor

@ten1seven ten1seven left a comment

Choose a reason for hiding this comment

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

I think this fulfills what the Figma images show but reading the ticket, I think there's a mismatch in spec versus images?

@bd-viget
Copy link
Contributor Author

I think this fulfills what the Figma images show but reading the ticket, I think there's a mismatch in spec versus images?

I thought so too, but I guess I assumed the Breadcrumbs were the "menu items" and the Company Logo was the image.

Also, it's usually a bit strange to specify the links in Breadcrumbs, usually the site map decides that automatically.

@bd-viget
Copy link
Contributor Author

@ten1seven @stephaniefois

I think the description for ticket #69 was copied from ticket #67 and just not updated.

@ten1seven
Copy link
Contributor

Yea, spec duplication seems to be the case.

@bd-viget bd-viget force-pushed the bd/69-page-header-component branch from c819001 to 07c1a12 Compare May 21, 2024 18:25
@bd-viget
Copy link
Contributor Author

Ok, when testing this, you may need to run composer install, WP Starter now includes RankMath, which is what we're using for breadcrumbs, but they need to be enabled.

Copy link
Contributor

@nathan-schmidt-viget nathan-schmidt-viget left a comment

Choose a reason for hiding this comment

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

Works great!

@bd-viget bd-viget force-pushed the bd/69-page-header-component branch from 7995fe2 to 699dd67 Compare May 23, 2024 20:55
@bd-viget bd-viget merged commit 5a60654 into main May 23, 2024
@bd-viget bd-viget deleted the bd/69-page-header-component branch May 23, 2024 20:56
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.

3 participants