I built a sandbox module creating a yoast_seo report utilizing a node preview. It simply creates a report by passing the rendered content to the yoast_seo library, there is no instant feedback on the form like in this modules implementation, instead the report is triggered by using a seo preview button. The major advantage of using the preview is, that you can easily include all different field types like paragraphs, nested fields, also it takes view mode settings into account.
If you are interested maybe we could find a way to combine the functionalities.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

volkerk created an issue. See original summary.

Kingdutch’s picture

Hi volkerk!

Cool! Thanks for the suggestion.

If you look at the latest development version of the 2.x version then you can actually see that I've implemented something similar by rendering the entity. This has the added benefit that it's not coupled to nodes which means the module can also be used with other entity types.

When I have some time, I'll check out your solution to see if we can improve what's in the module!

For now, some more work will be required before we can do a 2.0 alpha but if you could test what's in the 2.x development branch then that would be appreciated greatly!

~ Alexander

Kingdutch’s picture

Hi volkerk,

I took a look at your module and it's an interesting approach. My main gripe with it is that it doesn't work for entities other than nodes and that it won't work nicely together with other modules that try to take over the Node preview form handler.

There is definitely some things that can be learned from your Sandbox and used in the module though. For example we don't have a hook_help implementation in the RTSEO module yet and I think your metatags retrieval for some items (title, meta description) is a bit more elegant than the the current implementation. I also think that any alter hooks for metatags might currently not work for the previews of the Real-Time SEO module.

I'll leave this issue open but we should see if we can incorporate the above things in separate issues and close this meta-issue.

I look forward to hearing your thoughts. It would be cool if the 2.x version of this module could be used in Thunder.

~ Alexander

Kingdutch’s picture

Status: Active » Needs review
volkerk’s picture

Hi Kingdutch,

thank you for your feedback, I updated the sandbox module, it now works for all entities which implement the EditorialContentEntityType class, this covers node, taxonomy_term and media entities and should be future proof.

I had a look at the 8.x-2.x branch of the yoast_seo module, I see that you provide a preview functionality as well now, unfortunately it does not play well with the thunder distribution: #2927081: Preview does not work for paragraphs

Also I am not sure about doing the request handling and building the form_state yourself, may I suggest that we join our efforts, I would very much like to provide you a patch which incorporates the approach I used in the sandbox module into the ajax preview handling of the yoast_seo module.

Kingdutch’s picture

Hi Volkerk,

The preview not working for paragraphs is something I encountered as well. It seemed to work with the most basic paragraph but doesn't like some of the fields (my simplest case seems to be a paragraph with a dropdown).

My next step is to write a failing test in #2573899: Paragraphs module support and see if I can fix that.

Personally I think the way the AJAX request is handled is quite elegant. I think it can cover most any entity type (if we get the field value extraction fixed). I would at least like to make sure that the entity generation from form values and the move from entity to values that are usable by the JavaScript library are kept separate.

How would you suggest we approach it?

volkerk’s picture

Hi kingdutch,

you are right, we should definitely keep the EntityPreviewer service.

The problem with the recreation of $form and $form_state is, that some Widgets rely on the original form object to handle data.

What about this: we go with the form handler class to get a proper submit on the form object, but keep the previewer service for preparing the data for the JavaScript library?

Kingdutch’s picture

Assigned: Unassigned » Kingdutch
Status: Needs review » Needs work

Writing down a realisation I just had.

I believe the bug in the current implementation in the module is that validation logic is required to massage the values properly. Validation logic's execution depends on the button that created the form validation request. The current javascript logic that uses a separate route doesn't have a proper method to select the submit button as the triggering element. Once a paragraph element is added then the default selected button becomes one of paragraphs which causes some of the validation logic to malfunction. Adding a separate form handler with a corresponding button that can be used to trigger submission circumvents this.

I'm warming up to this approach more and more as it requires less reconstruction of Drupal core methods and solves the above problem more elegantly.

I'll see if I can extract the relevant parts from your sandbox today or tomorrow :)

Kingdutch’s picture

Assigned: Kingdutch » Unassigned
Status: Needs work » Needs review
FileSize
23.58 KB

I a little while later, I've come to the attached patch that I gave the following commit message:

This commit implements a form handler that uses the Drupal AJAX
framework to retrieve an analysis form and analyse its values. This
removes the necessity for a lot of routing hacks.

Many thanks to volkerk for the initial implementation of the handler
that he made. His work has been adapted to keep the analysis itself
in a separate class. The existing widgets have been altered to add
the submit button which has been hidden. The button is triggered by
the javascript Orchestrator object. The AJAX response has been
simplified to pass the new data back to the orchestrator which then
runs the analysis.

You can also see this on the feature/improved-preview branch that I pushed. I had begun writing a test but I forgot to push that work from my work laptop so the attached patch only implements the functionality using your sandbox as guide.

Please let me know what you think and if this works for you :)

Status: Needs review » Needs work

The last submitted patch, 9: yoast_seo-node-preview-sandbox-2917280-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Kingdutch’s picture

Status: Needs work » Needs review
FileSize
4.41 KB
24.91 KB

The configuration page test caught an error where there was still a reference to the route that was made obsolete. Attached is the fix for this. It also fixes the coding standards that were reported. There's also a slight change in the test to use absolute URLs but that shouldn't functionally change anything.

Status: Needs review » Needs work

The last submitted patch, 11: yoast_seo-node-preview-sandbox-2917280-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Kingdutch’s picture

Status: Needs work » Needs review
FileSize
24.9 KB
616 bytes

Whelp, that's embarrassing.

volkerk’s picture

Status: Needs review » Reviewed & tested by the community

Great work!
I have tested this in thunder and default drupal with paragraphs installed. Preview functionality works fine.

I had two nitpicks but I think we can address them later:

* #2935670: RTSEO.js library is too verbose.
* #2935681: Handlers are not attached

Anyway, I think this is a big improvement, thank you very much.

  • Kingdutch committed dc0cb54 on 8.x-2.x
    Issue #2573899 by volkerk, Kingdutch: Paragraphs module support
    
    This...
Kingdutch’s picture

Status: Reviewed & tested by the community » Fixed

I'm marking this issue as fixed. The latest dev commit of 2.x includes support for analysing nodes with Paragraphs. I've created a follow up for this issue to write tests but that's blocked by work in the paragraphs module: #2937665: Write a test for Paragraphs module support

Thanks for the help!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.