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 Thank with Google supporter wall widget #5451

Closed
felixarntz opened this issue Jun 27, 2022 · 27 comments
Closed

Implement Thank with Google supporter wall widget #5451

felixarntz opened this issue Jun 27, 2022 · 27 comments
Labels
Module: Thank with Google Thank with Google module related issues P0 High priority PHP Type: Enhancement Improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Jun 27, 2022

The TwG module will include a traditional WordPress widget for its "Supporter wall", which can be placed using WordPress's widgets screen. The widget UI depends on a JavaScript snippet being available, so this will also require relevant tweaks to the placement of that JavaScript snippet.

See #5450 for the code snippet that is potentially useful for QAing this issue. It is critical that usage of the widget is QAd for both the classic widgets screen (pre WP5.9) and the block-based widgets screen (since WP5.9).


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • A new Thank with Google Supporter_Wall_Widget class should be implemented, extending WordPress's WP_Widget.
  • It should only come with a single widget option for the title (standard like for other WordPress widgets).
  • It should render only the minimum WordPress widget markup (including title if set), plus a placeholder for the TwG supporter wall which will then be populated by the JS script from Implement Thank with Google tag placement infrastructure #5450. See the section below the main ACs for details on that placeholder.
  • The form where users can edit the widget options should just show a regular text field for the title (standard like for other WordPress widgets), and a description text: The color of the supporter wall is based on the color theme you selected in the Thank with Google module settings.
  • The widget should be registered from the main Thank_With_Google class (on widgets_init, as usual).
  • Now comes the tricky part: It needs to be ensured that the widget preview, relevant for the new block-based WordPress widgets UI from WP5.9+, works correctly. For this purpose, it may be necessary to somehow load the script that was added in Implement Thank with Google tag placement infrastructure #5450 within WP Admin, since otherwise the supporter wall placeholder from the widget may never be replaced with the actual supporter wall.

TwG supporter wall placeholder

  • The supporter wall placeholder snippet should be exactly as follows:
<div twg-thank-wall style="width: 100%; height: 490px; min-height: 150px; overflow: hidden; border: 1px solid #e3e3e3; border-radius: 15px; margin: 20px 0;"></div>

Implementation Brief

  • Create a new includes/Modules/Thank_With_Google/Supporter_Wall_Widget.php file with the corresponding Supporter_Wall_Widget class that extends the WP_Widget class. It should have the following method:
    • The constructor should call the parent constructor with the following arguments:
      • 'googlesitekit-twg-supporter-wall'
      • 'Thank with Google: Supporter Wall'
    • The form method should display the title field and the description text below the title: The color of the supporter wall is based on the color theme you selected in the Thank with Google module settings.
    • The update method should save the new title value.
    • The widget method should:
      • render the widget title if set and the placeholder snippet from the "TwG supporter wall placeholder" section in AC.
      • enqueue the google_thankjs script if registered.
  • Update the register method of the Thank_With_Google class to do the following if the module is connected:
    • register the new Supporter_Wall_Widget widget by adding a new hook for the widgets_init action that calls the register_widget function with the widget class name.
    • register a new hook for the admin_init action that calls the register_tag method if the current request has non-empty $_GET['legacy-widget-preview']['idBase'] parameter that equals to googlesitekit-twg-supporter-wall.
    • register a new hook for the rest_pre_dispatch filter that calls the register_tag method if the current request URL ends with widget-types/googlesitekit-twg-supporter-wall/render and the $request['id'] value equals to googlesitekit-twg-supporter-wall.

Test Coverage

  • N/A

QA Brief

  • Create a new instawp site.
  • Install the plugin and activate the twg module.
  • Follow the Setup instructions in the QA Brief for Implement Thank with Google tag placement infrastructure #5450 to connect the TwG module.
  • Go to the widgets page and add a new legacy widget where you need to select the supporter wall widget.
  • Verify that the preview is shown for the widget when the widget loses focus.
  • Verify that the widget displays the supporter wall on the frontend.

Changelog entry

  • Add new "Thank with Google: Supporter Wall" WordPress widget.
@felixarntz felixarntz added P0 High priority Type: Enhancement Improvement of an existing feature Module: Thank with Google Thank with Google module related issues labels Jun 27, 2022
@felixarntz felixarntz self-assigned this Jun 27, 2022
@felixarntz felixarntz removed their assignment Jun 27, 2022
@techanvil techanvil assigned techanvil and unassigned techanvil Jul 6, 2022
@eugene-manuilov eugene-manuilov self-assigned this Jul 8, 2022
@eugene-manuilov
Copy link
Collaborator

  • Now comes the tricky part: It needs to be ensured that the widget preview, relevant for the new block-based WordPress widgets UI from WP5.9+, works correctly. For this purpose, it may be necessary to somehow load the script that was added in Implement Thank with Google tag placement infrastructure #5450 within WP Admin, since otherwise the supporter wall placeholder from the widget may never be replaced with the actual supporter wall.

@felixarntz, can we extract this part for the modern block-based widget into a separate ticket since it will require a lot of work and considerations as to how to define custom blocks in our plugin, how to build them using webpack, and so on? I have defined the initial steps in this IB but I understand that I'll need to add much more and this ticket will be most likely 30+ which is not very good.

@felixarntz
Copy link
Member Author

felixarntz commented Jul 12, 2022

@eugene-manuilov There may be a misunderstand here, the block implementation for this widget should not be part of this issue (in fact not even part of the TwG MVP requirements). So I also think that with the classic widget itself this issue should only be a 7 or less, not a 19. We explicitly did not include any blocks in the V0 to avoid all this complexity you're speaking about.

The note in the ACs about widget previews working is only referring to the TwG JS snippet, which is needed for the preview to work. So I was thinking we should try to just inject the TwG JS snippet into the widgets admin screen, so that it would replace the twg-thank-wall div accordingly with the real UI. This is a bit hacky, but should be a lot more straightforward if it works. We could break that part out into a separate issue, in which case this here would only be the (very simple) WP_Widget implementation.

In any case, developing a block here is out of scope.

@felixarntz felixarntz removed their assignment Jul 12, 2022
@eugene-manuilov
Copy link
Collaborator

Ah... thanks for clarifying.... 🤦 I thought you wanted to build a preview for the new widgets block editor that requires widgets to be block based which is why I went with that block based approach.

The note in the ACs about widget previews working is only referring to the TwG JS snippet, which is needed for the preview to work.

Now I am wondering which preview do you mean then? Is it a preview of a post or something different?

@felixarntz
Copy link
Member Author

@eugene-manuilov

Now I am wondering which preview do you mean then? Is it a preview of a post or something different?

Have you seen the new widgets.php UI that was added in WordPress 5.9? Widgets now have a live preview right within the widgets editing screen. So we need to make sure that that works and actually shows the supporter wall. Normally this works for widgets out of the box, even classic widgets, but we have a special situation here since the supporter wall widget requires a script to be output (see #5450).

This wasn't a problem prior to WP 5.9 because there was no preview, but now we need to make sure it works in relation to the script that is inserted for TwG. The widget itself will not work without the script, so we need to ensure it's somehow added to the screen. In short, we should ensure that the implemented WP_Widget shows its preview reliably in:

  • Customizer prior to WP 5.9
  • Customizer since WP 5.9
  • widgets.php admin screen since WP 5.9

I think the Customizer preview should work out of the box, but even that we need to double-check. I'm sure to make the preview in the new Widgets admin UI work though we need to add some custom code to somehow inject the TwG script there.

@eugene-manuilov
Copy link
Collaborator

@felixarntz

Have you seen the new widgets.php UI that was added in WordPress 5.9? Widgets now have a live preview right within the widgets editing screen

Yes, I saw it for sure, just didn't notice that the legacy widget component renders the preview for a legacy widget when it becomes inactive / looses the focus. Will update IB tomorrow. Thanks a lot for clarifications and sorry for misunderstanding.

@felixarntz
Copy link
Member Author

@eugene-manuilov

Yes, I saw it for sure, just didn't notice that the legacy widget component renders the preview for a legacy widget when it becomes inactive / looses the focus. Will update IB tomorrow. Thanks a lot for clarifications and sorry for misunderstanding.

No worries, this was certainly a bit unclear and hard to explain. Hopefully we can figure out a simple way to make the preview work without some too crazy hacks :)

@felixarntz felixarntz removed their assignment Jul 12, 2022
@eugene-manuilov
Copy link
Collaborator

Thanks, @felixarntz. I have updated IB. I think it should be good now. Do you mind reviewing it?

@felixarntz
Copy link
Member Author

@eugene-manuilov Looks great - nice to see that it looks like getting the JS snippet to work in the preview will be easier than I thought! IB ✅

@felixarntz felixarntz removed their assignment Jul 15, 2022
@hussain-t hussain-t assigned hussain-t and unassigned hussain-t Jul 18, 2022
@eugene-manuilov eugene-manuilov self-assigned this Jul 19, 2022
@mohitwp mohitwp assigned eugene-manuilov and unassigned mohitwp Aug 3, 2022
@eugene-manuilov
Copy link
Collaborator

I added 3 widgets in footer. But on refreshing the page widget display behavior is changing. After refresh randomly widgets—Placeholders are getting hide.

@mohitwp we don't control what is shown when widgets are displayed on the frontend. The widget loads a javascript file for the TwG supporter wall and that script makes a decision of what should be shown.

@wpdarren
Copy link
Collaborator

wpdarren commented Aug 4, 2022

QA Update: ❌

@eugene-manuilov while testing #5462 I discovered a few potential issues to highlight.

  1. When I added the widget and then started to type the title, a lot of error messages appeared in the console. I am assuming that this is because the API is not connected yet, but wanted to check.

image

  1. The TwG block details need improving, i.e. it says 'Legacy widget' and the description is in uppercase.

image

  1. Possibly linked with the observation above. When you hover over the widget to delete it, it says 'Delete Legacy widget' I feel this should read 'Delete Thank with Google widget'

image

  1. When you switch to a full side editor theme, i.e. 2022, I am unable to find the TwG widget to add to my site. I tried adding the widget in a few different places.

image

c.c. @mohitwp since you are testing this ticket too - just found them while testing another ticket.

@wpdarren wpdarren assigned eugene-manuilov and unassigned mohitwp Aug 4, 2022
@eugene-manuilov
Copy link
Collaborator

@wpdarren

  1. When I added the widget and then started to type the title, a lot of error messages appeared in the console. I am assuming that this is because the API is not connected yet, but wanted to check.

Those errors are expected at the moment and caused by the TwG script. We can't fix it.

2. The TwG block details need improving, i.e. it says 'Legacy widget' and the description is in uppercase.

What you see in the sidebar is controlled by the Gutenberg editor and that's how it treats classical widgets. We can't fix it.

3. Possibly linked with the observation above. When you hover over the widget to delete it, it says 'Delete Legacy widget' I feel this should read 'Delete Thank with Google widget'

Same, we can't fix it.

4. When you switch to a full side editor theme, i.e. 2022, I am unable to find the TwG widget to add to my site. I tried adding the widget in a few different places.

Did you search for legacy widget?

@wpdarren
Copy link
Collaborator

wpdarren commented Aug 8, 2022

@eugene-manuilov

  1. When you switch to a full side editor theme, i.e. 2022, I am unable to find the TwG widget to add to my site. I tried adding the widget in a few different places.

Did you search for legacy widget?

I searched for Google but also legacy and nothing appears.

When I use the Twenty Twenty Two theme, with full-site editing, and try and add a widget, its not appearing. On an older theme that isn't full-site editing, the widget does appear when searching for Google.

I think this could be an issue unless I am missing where to add the widget?

widget.mp4

@eugene-manuilov
Copy link
Collaborator

I think this could be an issue unless I am missing where to add the widget?

@wpdarren looks like the legacy widgets component has been disabled for the new post editor intentionally, see a GH ticket for it: WordPress/gutenberg#24900. I think we shouldn't do anything with it until we create a new block-based widget for the supporter wall (which is currently is out of scope).

If users want to use the widget with FSE themes, they will need to install a plugin that activates the legacy widgets block there: https://wordpress.org/plugins/x3p0-legacy-widget/

cc @felixarntz @aaemnnosttv

@eugene-manuilov eugene-manuilov removed their assignment Aug 8, 2022
@wpdarren
Copy link
Collaborator

wpdarren commented Aug 8, 2022

@eugene-manuilov @aaemnnosttv @felixarntz this could cause problems for users who have FSE themes. I feel we need to create a ticket. I am sure @bethanylang and team will get support requests. So, if it's out of scope then we need to prepare communication for this.

We can move this ticket forward, but I'd welcome your opinions on this.

@eugene-manuilov
Copy link
Collaborator

I feel we need to create a ticket for this.

Yes, we definitely need a separate ticket for it.

@mxbclang
Copy link

mxbclang commented Aug 8, 2022

Thanks for flagging this, @wpdarren! Agreed that we'll definitely get support topics about this and it's certainly not ideal, and we should create a block-based widget in addition to the legacy widget.

That said, given that this feature will only be available for a small subset of users for now, I'm not sure if it's a blocker for initial launch, so curious what the rest of the team thinks.

@eugene-manuilov
Copy link
Collaborator

That said, given that this feature will only be available for a small subset of users for now...

This feature will be available for all users who use non-FSE themes on their sites. FSE themes are not widely spread yet, so it should be the other way around:

That said, given that this feature will only be available for a small the major subset of users for now...

@wpdarren
Copy link
Collaborator

wpdarren commented Aug 8, 2022

@bethanylang thanks for the input. I will create a ticket for this before I leave for the day.

@eugene-manuilov one final question (I hope) Is the widget set up to only work on one widget area? For example, if I add the widget to two widget areas in the example below, the widget outline only appears on one of the widgets. (widget 3) I'm just wanting to make sure that this isn't an issue if a user adds it on to the sidebar and then footer. Thoughts?

image

@mxbclang
Copy link

mxbclang commented Aug 8, 2022

@eugene-manuilov Yep, understood on the difference between FSE/non-FSE. I was referring more to the fact that this is an experimental feature and it's US only for now, as I understand it?

@eugene-manuilov
Copy link
Collaborator

eugene-manuilov commented Aug 9, 2022

@eugene-manuilov one final question (I hope) Is the widget set up to only work on one widget area? For example, if I add the widget to two widget areas in the example below, the widget outline only appears on one of the widgets. (widget 3) I'm just wanting to make sure that this isn't an issue if a user adds it on to the sidebar and then footer. Thoughts?

@wpdarren if you open the source code of the page, how many occurrences of the <div twg-thank-wall tag do you see there? If you see the same number of elements as you have widgets, than it works correctly, otherwise not.

@eugene-manuilov
Copy link
Collaborator

I was referring more to the fact that this is an experimental feature and it's US only for now, as I understand it?

@bethanylang Ah... yes, in this case you are correct, although I am not sure if we can consider US only users as a small subset though 😄

@eugene-manuilov eugene-manuilov removed their assignment Aug 9, 2022
@wpdarren
Copy link
Collaborator

wpdarren commented Aug 9, 2022

QA Update: ✅

Verified:

  • When a new legacy widget is added the preview is shown for the widget when the widget loses focus. I checked the front end and the support wall widget appears on the front end. The outline appears on the widget. I checked the code that the twg-thank-wall appears depending on the amount of widgets are added.

image

Note: No design is appearing yet due to the API. Also the legacy widget does not appear on a FSE theme like 2022, but a ticket has been raised to investigate this issue as it will cause user support tickets.

@wpdarren wpdarren removed their assignment Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Thank with Google Thank with Google module related issues P0 High priority PHP Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants