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

Blockbase: refactor footers to patterns #5245

Merged
merged 12 commits into from
Jan 13, 2022
Merged

Blockbase: refactor footers to patterns #5245

merged 12 commits into from
Jan 13, 2022

Conversation

MaggieCabrera
Copy link
Contributor

@MaggieCabrera MaggieCabrera commented Dec 22, 2021

Changes proposed in this Pull Request:

This is a WIP

This PR repeats the work done for headers in #5238 but for footers. We are removing duplicate code and giving the user options to use all possible footers designed between our themes no matter the theme you installed.

I have a couple of comments I'd like to discuss:

  • I've not wrapped the "Proudly Powered by Wordpress" texts for translation because we have the plugin on dotcom to do that for us. I think it would be safe to do it anyway, let me know what you think.
  • Should we make the pattern from Consider adding a search icon to the Geologist + Zoologist footers #5025 be the default footer for Geologist and Zoologist even though those are already launched? Or should we just keep them as a pattern that users can swap to if they want?
  • Skatepark has some text under the title:

Skatepark's coaches will work with you to develop and improve your skating abilities.

Should we change this text to something generic that works for all themes? Or maybe we should make this be the tagline block instead? @kjellr @beafialho I'd love your input on this

Seedlet blocks footer was very different from the original, I tried to make it closer to what it should be. Also ended up opening WordPress/gutenberg#37586 because it's related, I think it's fine if that doesn't get merged though.

Screen Capture on 2021-12-22 at 13-20-48

The new pattern on geologist with the small search block block style (I didn't apply it to the theme, just made the screenshot, this pattern is not used in any theme at the moment in this PR, pending discussion):

Screenshot 2021-12-22 at 13 42 05

Related issue(s):

I'm adding the extra pattern from #5025

Closes #5025

@MaggieCabrera MaggieCabrera requested review from kjellr, beafialho and a team December 22, 2021 12:42
@MaggieCabrera MaggieCabrera marked this pull request as ready for review December 22, 2021 12:42
@MaggieCabrera MaggieCabrera self-assigned this Dec 22, 2021
@kjellr
Copy link
Contributor

kjellr commented Jan 3, 2022

Should we change this text to something generic that works for all themes? Or maybe we should make this be the tagline block instead? @kjellr @beafialho I'd love your input on this

Yeah, the site tagline there is fine.

@MaggieCabrera
Copy link
Contributor Author

Rebased this one too. Changed the text on the skatepark pattern to the tagline.

@kjellr should we use the new pattern from #5025 on Geologist/Zoologist?

Copy link
Member

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

LGTM

@pbking pbking force-pushed the try-refactor-footers branch from bf8b3c1 to 3705f72 Compare January 13, 2022 16:26
@pbking pbking merged commit ff859ba into trunk Jan 13, 2022
@pbking pbking deleted the try-refactor-footers branch January 13, 2022 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding a search icon to the Geologist + Zoologist footers
4 participants