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

fix: verify delayed prompt can still be displayed before showing #1344

Merged

Conversation

chickenn00dle
Copy link
Contributor

@chickenn00dle chickenn00dle commented Sep 9, 2024

All Submissions:

Changes proposed in this Pull Request:

Closes https://app.asana.com/0/1207817176293825/1208253149260198/f

With ras-acc, it is now possible for segment conditions to change for the reader between when a delayed prompt is "queued" and when it is displayed. This PR adds a check for delayed prompts to ensure it should still be displayed before unhiding:

CleanShot 2024-09-08 at 14 31 15

How to test the changes in this Pull Request:

To test, be sure to check out epic/ras-acc in blocks, theme, newsletters, and plugin

  1. As admin, set up a registration block campaign for unregistered readers to render on every page or post view after a few seconds delay
  2. As a logged out reader, visit the any page or post and confirm the registration overlay appears as expected (I had to clear local storage before this worked for me)
  3. Refresh the page, and BEFORE the overlay appears quickly click the sign in button in the site header
  4. Complete either the registration flow, or the sign in flow until the modal closes
  5. On trunk the registration overlay will appear, but since you are signed in, you will see the success state of the registration block. On this branch, the overlay should no longer appear as the reader no longer matches the segment.
  6. As admin again, disable the registration overlay and enable a new overlay (with any type of content) that targets everyone and appears on every page view after a few seconds delay.
  7. Repeat steps 1-5, but this time verify the prompt DOES appear once the auth modal closes.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@chickenn00dle
Copy link
Contributor Author

This is technically a fix for ras-acc, however this is still a general enhancement that applies outside of ras-acc, so targeting trunk for this. But let me know if you'd like me to open a new epic branch for this instead!

Copy link
Contributor

@laurelfulford laurelfulford left a comment

Choose a reason for hiding this comment

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

Works as described! 🙌

I tested this against epic/ras-acc and it worked as described for both the registration prompt success message not showing, and a regular prompt showing after the RAS-ACC checkout was closed.

I also tested it against trunk, and the prompts kept working as expected (both for the registration, and a regular prompt).

@chickenn00dle chickenn00dle merged commit 235f36b into trunk Sep 9, 2024
9 checks passed
@chickenn00dle chickenn00dle deleted the fix/prevent-registration-popups-when-logged-in branch September 9, 2024 23:25
matticbot pushed a commit that referenced this pull request Sep 20, 2024
# [3.1.0-alpha.1](v3.0.1...v3.1.0-alpha.1) (2024-09-20)

### Bug Fixes

* check for non-preview logged in user on account criteria ([#1347](#1347)) ([9f6f062](9f6f062))
* prioritize ras overlays for delayed prompts ([21c3091](21c3091))
* prioritize ras overlays for delayed prompts [#1341](#1341)  ([c0027d1](c0027d1))
* verify delayed prompt can still be displayed before showing ([#1344](#1344)) ([235f36b](235f36b))

### Features

* store prompt activation date ([#1340](#1340)) ([2c11424](2c11424))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.1.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants