search

Yoast / yoast-acf-analysis

WordPress plugin that adds the content of all ACF fields to the Yoast SEO score analysis.
https://wordpress.org/plugins/acf-content-analysis-for-yoast-seo/
GNU General Public License v3.0
61 stars 20 forks source link

Add support for the new Link Field #98

Closed viktorfroberg closed 6 years ago

viktorfroberg commented 7 years ago

Summary

This PR can be summarized in the following changelog entry:

Fixes #78

kraftner commented 7 years ago

Most oft the issues @andizer pointed out are totally valid but probably due to the fact that I proposed just copying the URL Field which has the same issues. :) Worth fixing anyway. I've also opened #100 to address this on a global scale.

Another thing I'd like to add is that this PR is lacking tests.

viktorfroberg commented 7 years ago

Added some documentation, removed some unused variables and added some tests for the link field. But It fails on 3 field types (Image Field, Link Field, Repeater Field now, I have no idea why.

They all fail for the same reason, Timed out while waiting for element <body.post-new-php> to be present for 15000 milliseconds. - expected "visible" but got: "not found"

Got an idea @kraftner?

kraftner commented 7 years ago

I just ran the tests on the PR and for me everything is working.

The error you are seeing happens when login into WP failed for whatever reason. You should see screenshots of the failed state in tests/js/system/screenshots/*. Maybe this hints at the problem.

viktorfroberg commented 7 years ago

That's weird, all screenshots from that kind of error are broken, other screenshots are working. I'll try to find some time to debug this later today, but it looks like it's a problem with my setup.

kraftner commented 7 years ago

Sigh. That is the downside of those tests, this hard to debug errors...

I think I have seen that from time to time as well without any clear reason. What you can try is increasing the memory for the VM (see https://varyingvagrantvagrants.org/docs/en-US/vvv-config/#vm_config)

vm_config:
  memory: 2048

which has apparently made things more stable here.

moorscode commented 6 years ago

I have resolved the merge conflicts and made sure the JavaScript conforms to the standards introduced in develop.