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

Buyer's Guide og tags & main nav share icons #1926

Merged
merged 15 commits into from
Oct 18, 2018
Merged

Conversation

mmmavis
Copy link
Collaborator

@mmmavis mmmavis commented Oct 12, 2018

Related #1899

https://foundation-mofostaging-pr-1926.herokuapp.com/privacynotincluded/

Only covers the following

Not covered in this PR

  • creep-o-meter related share

❗️ Remember to revert changes in network-api/networkapi/buyersguide/views.py before merging

@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1926 October 12, 2018 21:23 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 12, 2018 21:33 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 12, 2018 22:23 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 16, 2018 21:58 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 17, 2018 20:00 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 17, 2018 23:17 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 17, 2018 23:26 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 17, 2018 23:53 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 18, 2018 02:19 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 18, 2018 06:19 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 18, 2018 06:37 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 18, 2018 06:37 Inactive
@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 18, 2018 07:11 Inactive
@mmmavis mmmavis changed the title (WIP) Buyer's Guide og tags Buyer's Guide og tags & main nav share icons Oct 18, 2018
@kristinashu
Copy link

Yay looks great!

Copy link doesn't work on my iPhone but that is an issue on the live site and also on the category page so we can try to sort that out later.

@alanmoo
Copy link
Contributor

alanmoo commented Oct 18, 2018

Let's actually go ahead and leave the removal of the login prevention when we merge this since we want this to be unlocked today anyway.

@@ -30,7 +30,7 @@ def get_average_creepiness(product):

# Login required so we can continue to develop this and merge into master without the public seeing it.
# Remove this when we're ready to launch.
@login_required
# @login_required
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# @login_required
@login_required

@@ -41,7 +41,7 @@ def buyersguide_home(request):
})


@login_required
# @login_required
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# @login_required
@login_required

@@ -53,7 +53,7 @@ def category_view(request, categoryname):
})


@login_required
# @login_required
Copy link
Contributor

@alanmoo alanmoo Oct 18, 2018

Choose a reason for hiding this comment

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

Suggested change
# @login_required
@login_required

@@ -64,7 +64,7 @@ def product_view(request, productname):
})


@login_required
# @login_required
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# @login_required
@login_required

Copy link
Contributor

@alanmoo alanmoo left a comment

Choose a reason for hiding this comment

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

Scratch my previous comment- keep the login required bits (since testing fails due to non-use of that decorator)

@mmmavis mmmavis temporarily deployed to foundation-mofostaging-pr-1926 October 18, 2018 18:49 Inactive
@alanmoo alanmoo merged commit 45c684c into master Oct 18, 2018
@alanmoo alanmoo deleted the issue-1899-bg-og-tags branch October 18, 2018 19:49
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.

4 participants