Site Credit Block Structure

[noahtallen]

For now, I'm looking for feedback on if this looks like the right direction for the block. We'll continue using the site option to store the data on dotcom, but the main changes we'll need (for the future) are adding integration points to override settings in a dotcom context (i.e. hooks for whether or not the block is enabled, messages to display prompting for an upgrade, etc)

To do for this PR:

• Allow users to align the block left/right/center
• Use the footercredit option for dynamic rendering in PHP.
• Update Varia to not render a footer credit if FSE is enabled. (i.e. relying on the block instead)
• (possibly for followup) Refactor some of the useSiteOptions code so it doesn't need to have so many duplicated props.

Changes proposed in this Pull Request

• Create site credit block files and initial structure.
• Register footercredit as a setting so we can access it from the API. This is the same option name used to store the data currently on .com, so we'll be able to integrate easily with the existing solution.

Current functionality:

Note: rendering based on the selection isn't working yet. For now, test the block editor code.

Testing instructions

• Pull this, insert the site credit block, and play around with it. Right now, the option is persisted correctly, but it is not rendered.
• Look at the code.
• If you insert the credit block into the footer and also use this theme PR: (DON'T MERGE) Only display default credit info if FSE is not active. themes#1612, only the block version should show on the front end.

Resolves #37045.

[matticbot]

Test live: https://calypso.live/?branch=feature/site-credit-block

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[noahtallen]

As a bonus feature, I added alignment options so it could fit with whatever design the user may want to have. Let me know if we want to keep this

[mattwiebe]

This looks generally good so far. I find the UX of picking a logo but only seeing it once the block loses focus a bit weird. Maybe the dropdown should be an extra line underneath, not directly in the same place as where the logo/text would go?

[mattwiebe]

This whole area is a bit rough from a i18n perspective. It looks like you've lifted it directly from Varia, which would normally be a fine idea but they've made some mistakes there. I won't go too deeply into it since this is likely just placeholder stuff for now?

[noahtallen]

Yep, you're right, I copied from Varia. :) This should be slightly updated now, but I didn't change much in this area. I need to come up with a more robust way to handle all of the possibilities.

[noahtallen]

I updated to use the normal alignment buttons (not text align, block align) so that we can use the existing alignment classes on the front end. This seems to work pretty well, but some block editor interactions for right/left alignment are a bit ugly out of the box. I imagine this will improve with time.

@mattwiebe Hm, yeah that's a good point. I followed the existing design in Dotcom-roadmap/issues/50-gh, but I'm not sure if there should be a way to preview it as you select options? cc @shaunandrews

[Copons]

I think we should move the useSiteOptions refactor into its own PR, since it takes basically half of this one, and affects other FSE blocks.

[Addison-Stavlo]

The block structure is looking good!

However I am finding that the selected option gets reset when drag-and-drop is used into or out of a parent block. That is (as outlined in the gif below), if we select the logo option and then move the block to a different parent the option resets to the default text credit.

On another note, I am curious if this left-most option on the toolbar is showing the correct icons. At first it is a WordPress logo with a dropdown arrow, but on mouse-over it turns into the clockwise double arrow without a dropdown arrow (which when clicked does initiate a dropdown menu). Is that intended? I am a bit lost on what either of those logos are supposed to represent here.

[noahtallen]

the option resets to the default text credit.

@Addison-Stavlo Good catch, that's really weird! I'll take a look.

I am curious if this left-most option on the toolbar is showing the correct icons.

Yep, this is expected :) Gutenberg uses this to show the normal block icon, but then if you mouse over it, it shows the "transform" button. This is used to turn the block into a different block - for example, changing a heading to a paragraph block. In this case, if you chose "group" block, it should create a group block and then put the site credit block inside of it.

I think we should move the useSiteOptions refactor into its own PR

@Copons that's fair. I wanted to be able to use the new HOC here because it makes things so much easier, but probably good to review separately.

[noahtallen]

the option resets to the default text credit.

@Addison-Stavlo really good catch here! I was able to replicate it with existing blocks as well, so this is not related to changes here. It looks like the block resets when you drag it into the group, but the option hasn't been persisted to the database yet. I created an issue here: #37260

[noahtallen]

@Copons I have moved the site option changes to #37255 and have changed this PRs base to that branch so it can use the new function it introduces. (Should also make reviewing easier.) If that PR gets merged, I'll change the base of this branch back to master.

[noahtallen]

Tried to rebase, which turned out to have way to many conflicts, so a simple merge has gotten us back to master as the base branch.

[Addison-Stavlo]

Good Stuff!

I had a few questions about the code but nothing big or show stopping except the below noted testing issue.

I found an inconsistency between testing locally and on the dotcom sandbox. That is, if we go to update a wp_template_part (header or footer) and add the credit block, select the logo option, "update", and then "go back to page" we can see that:

• in the case of local env the logo save is respected.
• in the case of dotcom sandbox the block turns back to the default text setting.

It seems a bit different than the issue I noted earlier in the settings resetting when changing parent elements, something may not be right with the options saving on dotcom? 🤔

Local:

Sandbox:

[noahtallen]

in the case of dotcom sandbox the block turns back to the default text setting.

Hm, that is an odd isue, I'll have to look into that.

[shaunandrews]

The Calypso.live link doesn't seem to be working for this PR (I think its a FSE thing unrelated to this specific PR), but based on the images/GIFs above this all looks fine to me.

I do question the need for the , (comma) between the site title and the credit — especially as we visually differentiate them already — but thats a very minor things that shouldn't block this from merging.

[vindl]

As discussed on Slack, I think it's fine to merge this now provided that we remove the block registration.

Making this on Dotcom will require prerequisite Core changes to land. Landing this PR now will make follow up work easier, since we won't have to spend time keeping it up to date and rebasing.