-
Notifications
You must be signed in to change notification settings - Fork 49
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
feat(memberships): overlay style for content gate #2377
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working great! I ran into one JS error in the editor, and have a few questions below. I also wanted to mention that on a larger screen it's very easy to read the content under the overlay. Wonder if there's anything we can do to mitigate that beyond adding a "full-width" option for the gate and a configurable overlay opacity (both suggested below)?
Could we somehow set the CSS height of html
so that the page won't scroll past a certain point beyond where the gate is triggered? Or truncate the content under the overlay like we do with the inline gate?
const overlaySizes = [ | ||
{ value: 'small', label: __( 'Small', 'newspack' ) }, | ||
{ value: 'medium', label: __( 'Medium', 'newspack' ) }, | ||
{ value: 'large', label: __( 'Large', 'newspack' ) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
01e1ff7 tweaks the sizes so it reflects the available set in Newspack Campaigns and full width.
Since large is rather large, I set the default to medium now.
The content should be truncated with the same settings as applied to the inline gate. Did it not on your instance? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content should be truncated with the same settings as applied to the inline gate. Did it not on your instance?
Oh, you're right, it is! I just didn't notice because my article content is all lorem ipsum. 😆 Nit: Could we append an ellipsis [...]
to the end of the truncated content to indicate that it's truncated?
I also left some small suggestions which would apply a feature from the Campaigns plugin for PositionControl
selector when the overlay is full-width. After that, this looks good to go!
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
Nice! Thank you for the suggestion! Did you manage to confirm the |
61382e0 adds ellipsis when in overlay and fixes a minor bug. The last paragraph of the truncated content was missing a closing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the revisions!
# [1.112.0-alpha.3](v1.112.0-alpha.2...v1.112.0-alpha.3) (2023-04-04) ### Bug Fixes * add pre push hook ([#2395](#2395)) ([04a6e57](04a6e57)) * Avoid falal error on Reader Revenue wizard ([#2382](#2382)) ([646d212](646d212)) ### Features * **memberships:** overlay style for content gate ([#2377](#2377)) ([dd2ff5c](dd2ff5c))
🎉 This PR is included in version 1.112.0-alpha.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.112.0](v1.111.1...v1.112.0) (2023-04-10) ### Bug Fixes * add pre push hook ([#2395](#2395)) ([04a6e57](04a6e57)) * Avoid falal error on Reader Revenue wizard ([#2382](#2382)) ([646d212](646d212)) * bail silently when recaptcha key is not set ([#2363](#2363)) ([de18369](de18369)) * don't show gate unless attached to a specific post ([#2400](#2400)) ([c45097c](c45097c)) * if My Account is set to shown in RAS, show in Customizer at all breakpoints ([#2379](#2379)) ([1052923](1052923)) * **memberships:** remove content filters from excerpt ([#2398](#2398)) ([987df5b](987df5b)) * **stripe-sync-script:** process only customers with successful transactions ([#2355](#2355)) ([1020663](1020663)) ### Features * **amp-deprecation:** polyfill amp-vimeo tag ([#2372](#2372)) ([4a70b65](4a70b65)) * **amp:** polyfill lightbox effect ([#2324](#2324)) ([b31288c](b31288c)) * **memberships:** content gate page ([#2366](#2366)) ([15026ab](15026ab)) * **memberships:** overlay style for content gate ([#2377](#2377)) ([dd2ff5c](dd2ff5c)) * **popups:** support disabled segments ([#2376](#2376)) ([bfd65b2](bfd65b2))
🎉 This PR is included in version 1.112.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Implements overlay style options for the content gate page:
When selected, the overlay appears once the beginning of the post content reaches half the viewport. The settings for the number of visible paragraphs/more block do not change:
Gravacao.de.Tela.2023-03-29.as.16.47.12.mov
Overlay options are:
On mobile, the overlay will always be placed at the bottom and full-width.
For later discussions: some ads are rendered behind this overlay and we should probably have a way around this. One possibility, for a separate PR, is to allow the publisher to tweak visible ad placements in gated content, just like it's done for regular pages/posts:
How to test the changes in this Pull Request:
newspack-blocks
(for this)Other information: