Skip to content

Conversation

oredavids
Copy link
Contributor

@oredavids oredavids commented Jan 3, 2022

This PR adds a modal popup that will display to all users when they click on a link that takes them to github.com/gruntwork-io/*. The modal will warn users about getting 404s if they are not subscribers. There's also the ability to indicate that the warning should never be shown again.

@netlify
Copy link

netlify bot commented Jan 3, 2022

✔️ Deploy Preview for pensive-meitner-faaeee ready!

🔨 Explore the source changes: f96d6f6

🔍 Inspect the deploy log: https://app.netlify.com/sites/pensive-meitner-faaeee/deploys/61e0a5fa43c0d90007df0a49

😎 Browse the preview: https://deploy-preview-119--pensive-meitner-faaeee.netlify.app

@oredavids oredavids changed the title Subscriber notification for private Github links [WIP]: Subscriber notification for private Github links Jan 3, 2022
@ebeneliason ebeneliason force-pushed the subscriber-notification branch from 09edc2c to efddf16 Compare January 13, 2022 21:54
@eak12913 eak12913 changed the title [WIP]: Subscriber notification for private Github links [APT-1573] [APT-1574]: Subscriber notification for private Github links Jan 13, 2022
@oredavids
Copy link
Contributor Author

@eak12913 I noticed that when I clicked the checkbox(to prevent future notifications) and then hit the "Cancel Option"; I got the notification on the next link that I clicked. This is minor but shouldn't we always respect the notification preference?

Comment on lines +33 to +34
// If the user checked to never see this notice but subsequently cancels we will disregard their selection. We will
// only stop showing this notice if they check the box and then proceed to GitHub
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that this is intentional - Is there a particular reason for disregarding it?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can discuss this quickly next time we speak - but for the purpose of this PR we do want to have that behavior. It's a slightly better user experience as it applies your wishes only after you've chosen to follow the link forward. Otherwise you can get into a s situation where you dind't mean to dismiss the thing and never see it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍🏾

Copy link
Contributor

@ebeneliason ebeneliason left a comment

Choose a reason for hiding this comment

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

In the future we should refactor this so that we have a generic modal that's styled for our purposes, but let's get this in as-is.

@eak12913 eak12913 merged commit 97d6ca4 into master Jan 14, 2022
@eak12913 eak12913 deleted the subscriber-notification branch January 14, 2022 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants