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

Add proper error <ul> so errors can be properly shown #803

Merged
merged 1 commit into from
Sep 27, 2021

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Sep 24, 2021

Description of the Change

In our templates that output the connection dropdown, used when Pushing content, we were missing an unordered list element that we look for when outputting error messages. It appears this accidentally got removed when changes were made to better support AMP.

Without this element, if an error occurs during pushing, that error message will not be shown and a console error will happen (as reported in #795). This PR fixes this by adding that error element back to each template that needs it.

Alternate Designs

None

Benefits

Error messages will properly show and no console errors will happen

Possible Drawbacks

None

Verification Process

Easy way to create an error during push is to modify the ajax_push function and at the top of that, temporarily add a wp_send_json_error. Then go and try pushing some content. An error message should show and it should match whatever message as manually added.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Closes #795

Changelog Entry

Fixed - ensure error messages are shown properly if an error happens during a push

@jeffpaul jeffpaul merged commit 5b009d2 into develop Sep 27, 2021
@jeffpaul jeffpaul deleted the fix/error-output branch September 27, 2021 13:33
@Drmzindec
Copy link

Thank you for this update. The issue was that the original user who authorized the request was removed from the 1 site and the plugin couldnt verify. This update helped solve the problem. @dkotter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: can't access property "innerText", T is null
4 participants