Skip to content
This repository has been archived by the owner on May 29, 2020. It is now read-only.

Remove update time text after manual update #43

Merged

Conversation

bookdude13
Copy link
Contributor

Fixes #28.

Note that in #28 the issue disappeared after a refresh, since the page reloading set the text properly. I didn't think a page reload made sense after an update (especially since multiple updates may occur with bulk actions), so I added a jQuery hook. If there are coding standard changes, or a better place to put this, let me know and I can update it. I'm always ready to learn 😄

@@ -9,7 +9,7 @@
Tested up to: 5.4
Author: The WordPress Team
Author URI: https://wordpress.org
Contributors: wordpressdotorg, audrasjb, whodunitagency, pbiron, xkon, karmatosed, mapk
Contributors: wordpressdotorg, audrasjb, whodunitagency, pbiron, xkon, karmatosed, mapk, bookdude13
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don’t directly add your name to the contributors on the plugin header.
You can add your name on the readme.md file and I will take care your name is added to the props list of WordPress 5.5, but the plugin’s list of contributors is updated manually.

Thanks for your comprehension :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, and understood :) I thought I saw something similar in a PR here, but must've been mistaken.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much :)

audrasjb
audrasjb previously approved these changes Mar 10, 2020
Copy link
Collaborator

@pbiron pbiron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The </span> should be output before the <br />.

Copy link
Collaborator

@pbiron pbiron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the text to the empty string works. but I think it would be better to just remove the span (e.g., plugin_auto_update_time_text.remove().

@audrasjb audrasjb self-requested a review March 13, 2020 21:57
@audrasjb audrasjb dismissed their stale review March 13, 2020 21:58

Dismissing review as per @pbiron’s comment

@bookdude13
Copy link
Contributor Author

it would be better to just remove the span

I agree, removed.

The </span> should be output before the <br />.

If this happens then there is a gap between the auto-updates enabled text and the enable/disable link (see Hello Dolly):
gap_with_br

I could try to use other selectors to remove that <br /> in the script separately, but in my eyes the <br /> is part of the text I'm trying to select with the <span>. If that's just bad practice though let me know.

@pbiron
Copy link
Collaborator

pbiron commented Mar 14, 2020

If this happens then there is a gap between the auto-updates enabled text and the enable/disable link (see Hello Dolly):

You're right, my bad...I reviewed too quickly.

@audrasjb I think this is good to go now.

@audrasjb
Copy link
Owner

Yeah, it's good to go. Thank you @bookdude13 👍

@audrasjb audrasjb merged commit e18c373 into audrasjb:master Mar 14, 2020
@audrasjb audrasjb added this to the 0.3.0 milestone Mar 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forcing update while scheduled doesn't remove scheduled update
3 participants