Feature Request/Implementation

Add option to customize ' do not show again' cookies.

Proposed resolution

Add global setting to set days of expiration for a 'do not show again' cookie. Value 0 generates a session cookie.
Add fields to customize each modal, taking priority over the global setting.
Fall back to a high number (current) if none of the above can be found.

User interface changes

Added cookies details on modal, under 'enable do not show again' option.
Added cookies settings to module config page.

Issue fork modal_page-3267943

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

luiscarvalho created an issue. See original summary.

luiscarvalho opened merge request !17

luiscarvalho’s picture

luiscarvalho commented
Status: Active » Needs review
victorml’s picture

victorml commented
Status: Needs review » Reviewed & tested by the community

I tested and reviewed the code.

Everything worked fine. The cookies were created according to the settings applied.

The automated tests are passing too.

Seems fine to me.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Reviewed & tested by the community » Needs work

MR needs rebase and we should DI as @RenatoG pointed.

sophiavs’s picture

sophiavs commented
Assigned: luiscarvalho » sophiavs

I'll do the rebase

sophiavs’s picture

sophiavs commented
Assigned: sophiavs » Unassigned
Status: Needs work » Needs review
vitorbs’s picture

vitorbs commented

I'll review this one.

vitorbs’s picture

vitorbs commented
Assigned: Unassigned » vitorbs
mpaulo’s picture

mpaulo
He/Him
commented
Assigned: vitorbs » mpaulo

Reassigning for review ;)

mpaulo’s picture

mpaulo
He/Him
commented
Status: Needs review » Reviewed & tested by the community

Merged 4.1.x from upstream into the MR branch.

LGTM. Didn't find CS errors, either.

mpaulo’s picture

mpaulo
He/Him
commented
Assigned: mpaulo » Unassigned
jsricardo’s picture

jsricardo commented
Assigned: Unassigned » jsricardo
jsricardo’s picture

jsricardo commented
Assigned: jsricardo » Unassigned
mpaulo’s picture

mpaulo
He/Him
commented
Status: Reviewed & tested by the community » Needs review

As I've commited a merge from origin, I really shouldn't have RTBCed this one.

jsricardo’s picture

jsricardo commented
Assigned: Unassigned » jsricardo
jsricardo’s picture

jsricardo commented
Assigned: jsricardo » Unassigned
Status: Needs review » Reviewed & tested by the community

Tested in modal.
I enabled the option to customize cookie expiration date, disabled modal customization and modified the global expiration date.
All successful tests

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Reviewed & tested by the community » Needs work

MR needs another rebase.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Parent issue: » #3288686: Automated Drupal 10 compatibility fixes
mpaulo’s picture

mpaulo
He/Him
commented
Assigned: Unassigned » mpaulo

I'll try rebasing this one.

mpaulo’s picture

mpaulo
He/Him
commented
Assigned: mpaulo » Unassigned
mpaulo’s picture

mpaulo
He/Him
commented
Assigned: Unassigned » mpaulo

New tests miss new parameters added to some service classes on the DI refactoring of this issue.
I'll try to fix them.

mpaulo’s picture

mpaulo
He/Him
commented
Assigned: mpaulo » Unassigned
Status: Needs work » Needs review
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Needs review » Needs work
mpaulo’s picture

mpaulo
He/Him
commented
Status: Needs work » Needs review

I've resolved the MR issues.

diegors’s picture

diegors
He/Him
Primary language Portuguese, Brazil
Location Campinas - Brazil
commented
Assigned: Unassigned » diegors

I will review it.

diegors’s picture

diegors
He/Him
Primary language Portuguese, Brazil
Location Campinas - Brazil
commented
Assigned: diegors » Unassigned
Status: Needs review » Reviewed & tested by the community

The patch works as expected, changing the expiration time of the cookie please_do_not_show_again_modal_id is set according to the settings.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Reviewed & tested by the community » Fixed

Thanks!

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Parent issue: #3288686: Automated Drupal 10 compatibility fixes » #3307178: Plan to next Modal's release

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.