w3guy / persist-admin-notices-dismissal

Simple plugin that persists dismissal of admin notices across pages in WordPress dashboard.
http://w3guy.com/wordpress-admin-notices-dismissible/
90 stars 22 forks source link

Extra dismiss links #29

Closed lkoudal closed 3 years ago

lkoudal commented 4 years ago

Minor changes to .js file allows you to add links inside the div itself to dismiss the notification, not only by clicking the close button.

Any element with class "dismiss-this" can be clicked and will close the div.

afragen commented 4 years ago

Just curious why you this is needed. The dismissal is quite obvious using the standard WP core class.

lkoudal commented 4 years ago

Not so much needed as a little addition to a helpful library. Thank you :-)

I had a few users asking how to get rid of the notification.

image

Although I agree with you it is obvious once you know where the close button is, I listen to the feedback and I added the functionality in my own fork. I thought it might be helpful to others.

afragen commented 4 years ago

We'll see what @collizo4sky says about this.

w3guy commented 4 years ago

@afragen I have personally encountered a situation where users complained notices are not getting dismissed permanently. Turns out other plugin authors were also listening to close button click event and somehow preventing PanD from working.

I am in support of this. If you can test this PR to ensure everything works, that would be helpful and I will happily merge this.

Good work @lkoudal.

Remzi1993 commented 4 years ago

Hi @w3guy and @afragen

I really hope this feature gets implemented. I also want to use this to add another close button. Some user were indeed complaining that it was not obvious.

Kind regards, Remzi

afragen commented 4 years ago

@Remzi1993 I'm trying to test but I don't know what to use in the new link. Example or an example plugin would be great.

lkoudal commented 4 years ago

@afragen I can pitch in here - create an a href anywhere inside the container, and give it the class "dismiss-this"

<a href="#" class="dismiss-this">Dismiss warning for 24 hours.</a>

Yes, I am aware that naming could be better - feel free to change.

The length of the dismissal is still set as before. The JS code reads from the nearest (hierarchically) div with the data-dismissible attribute.

lkoudal commented 4 years ago

Full HTML example:

<div data-dismissible="dismiss-vulnerabilities-notice-1" class="notice notice-error is-dismissible">
    <h3><span class="dashicons dashicons-warning"></span> A warning</h3>
    <p><a href="#" class="dismiss-this">Dismiss warning for 24 hours.</a></p>
</div>
afragen commented 4 years ago

Thanks @lkoudal. Testing.

afragen commented 4 years ago

I have an issue. I primarily use this library with the afragen/wp-dependency-installer library. The wp-dependency-installer library adds a link that allows for installing/activating and the close box functions. When I have the extra dismissal link the install/activate link, which is JS, doesn't work as designed.

What's supposed to happen when I click the install/activate is that the action happens and a message is returned to the notice window. The action happens but there's no indication that it happened.

Remzi1993 commented 4 years ago

@lkoudal Could you help @afragen out? Since you know your code the best maybe you could solve his issue, so we can implement this.

lkoudal commented 4 years ago

@Remzi1993 I will take a look when I can - the conflict is in a 3rd party library I do not use.

afragen commented 4 years ago

wp-dependency-installer has a composer requirement for persist-admin-notices-dismissal. There must be something in the PR that interferes with the continuing function of other javascript in the notice.

Remzi1993 commented 4 years ago

@lkoudal see @afragen's reply, maybe his info helps.

w3guy commented 4 years ago

If we are able to test that this work. Before merging, the "dismiss-this" should be unique. I'm proposing "pand-dismiss-this".

w3guy commented 3 years ago

This looks good to me. Any objection @afragen ?

afragen commented 3 years ago

Let me test again. I should be able to do this today.

afragen commented 3 years ago

It seems to be working without conflict. Good job.

w3guy commented 3 years ago

@afragen thanks for testing. I will merge and release an update.

Many thanks to @lkoudal

afragen commented 3 years ago

It doesn't seem to be letting the notice stay dismissed in my testing.

afragen commented 3 years ago

This may simply be my method of integration in a test.

afragen commented 3 years ago

I think it was my testing method, the update doesn't seem to effect existing dismissal notices. Good work.