search

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/
87 stars 22 forks source link

optional: store in setting? #17

Closed futtta closed 5 years ago

futtta commented 5 years ago

follow-up to https://github.com/collizo4sky/persist-admin-notices-dismissal/issues/16; wouldn't it make sense to allow the dismissal to be stored in a setting iso a transient? :-)

afragen commented 5 years ago

It would solve some issues with object caching but also will leave entries to clutter up your options table. As a library there is no uninstall.

barisunver commented 5 years ago

The plugin (which uses this lib) should have the responsibility to include it in its uninstall.php tough, right?

afragen commented 5 years ago

@collizo4sky I can create a PR to save to the options table. I have code already for this.

What do you think?

w3guy commented 5 years ago

@afragen would be a great idea. I think initially it was saving to an option table when I created.

Go for it.

afragen commented 5 years ago

@barisunver ideally yes but we know that’s not likely to happen. It’s not likely to be many entries.

@collizo4sky will do. I just need to get off a plane first 😉

w3guy commented 5 years ago

@barisunver it will be just a row in the option table. What could possibly go wrong with not removing it?

barisunver commented 5 years ago

@barisunver it will be just a row in the option table. What could possibly go wrong with not removing it?

I wouldn't worry about that row at all, but recommending the plugin/theme dev to make it removable is always a better practice.

afragen commented 5 years ago

I have the code to remove it too. I’ll put those instructions in the README.

futtta commented 5 years ago

great, me happy! :-)

On Fri, Oct 19, 2018 at 3:47 PM Andy Fragen notifications@github.com wrote:

I have the code to remove it too. I’ll put those instructions in the README.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/collizo4sky/persist-admin-notices-dismissal/issues/17#issuecomment-431368906, or mute the thread https://github.com/notifications/unsubscribe-auth/AALEMQ5uMbZzB7cA_rjQLVTHTzxmX4UUks5umdgLgaJpZM4XwVu0 .

afragen commented 5 years ago

Anyone care to test before I submit a PR? https://github.com/afragen/persist-admin-notices-dismissal/tree/set-option

futtta commented 5 years ago

I will today/ tomorrow if time permits :)

On Sat, Oct 20, 2018 at 12:30 AM Andy Fragen notifications@github.com wrote:

Anyone care to test before I submit a PR? https://github.com/afragen/persist-admin-notices-dismissal/tree/set-option

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/collizo4sky/persist-admin-notices-dismissal/issues/17#issuecomment-431517622, or mute the thread https://github.com/notifications/unsubscribe-auth/AALEManyV_s4F6wHKetdb-PZFscjFtw9ks5umlKXgaJpZM4XwVu0 .

futtta commented 5 years ago

Afraid I have not had time and I'm leaving on holiday soon, so don't let me hold the PR/ merge back guys! ;-)

On Sat, Oct 20, 2018 at 10:12 AM frank goossens futtta@gmail.com wrote:

I will today/ tomorrow if time permits :)

On Sat, Oct 20, 2018 at 12:30 AM Andy Fragen notifications@github.com wrote:

Anyone care to test before I submit a PR? https://github.com/afragen/persist-admin-notices-dismissal/tree/set-option

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/collizo4sky/persist-admin-notices-dismissal/issues/17#issuecomment-431517622, or mute the thread https://github.com/notifications/unsubscribe-auth/AALEManyV_s4F6wHKetdb-PZFscjFtw9ks5umlKXgaJpZM4XwVu0 .