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

PNI Scaffolding #1804

Merged
merged 15 commits into from
Sep 11, 2018
Merged

PNI Scaffolding #1804

merged 15 commits into from
Sep 11, 2018

Conversation

gvn
Copy link
Contributor

@gvn gvn commented Sep 6, 2018

@alanmoo – This is obviously a work in progress, but I figure it's better to land in chunks (and safe since it's non-visible to the public and fairly sandboxed).

@gvn gvn requested a review from alanmoo September 6, 2018 23:20
@cadecairos cadecairos temporarily deployed to foundation-mofostaging-pr-1804 September 6, 2018 23:20 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1804 September 6, 2018 23:22 Inactive
@gvn
Copy link
Contributor Author

gvn commented Sep 7, 2018

Whoops forgot @alanmoo is out.

@Pomax can you review?

@gvn gvn requested review from Pomax and removed request for alanmoo September 7, 2018 18:08
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1804 September 7, 2018 23:19 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1804 September 10, 2018 16:43 Inactive
@gvn gvn temporarily deployed to foundation-mofostaging-pr-1804 September 10, 2018 21:24 Inactive
@Pomax
Copy link
Contributor

Pomax commented Sep 11, 2018

Interesting bit of scrollbar on windows:

image

</footer>
</body>
</html>
<script src="/_js/bg-main.compiled.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

technically this is invalid HTML since nothing's allowed to be between </body></html> or after </html> - given the current state of browsers, could we just move it to before </body> and mark it as defer so that it doesn't actually load until after the DOM has been fully parsed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't matter, but sure.

<div class="creepometer mb-5"></div>

<img src="{{mediaUrl}}{{product.image}}" width="500"/>
<div>
Copy link
Contributor

@Pomax Pomax Sep 11, 2018

Choose a reason for hiding this comment

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

It feels like we've hit upon one of those few legitimate uses for <table> here =P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly!

@@ -16,3 +16,8 @@ def buyersguide_home(request):
def product_view(request, productname):
product = Product.objects.get(name__iexact=productname)
return render(request, 'product_page.html', {'product': product, 'mediaUrl': settings.MEDIA_URL})


@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.

👍

@gvn gvn temporarily deployed to foundation-mofostaging-pr-1804 September 11, 2018 21:16 Inactive
Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

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

🚢 🇮🇹

@gvn gvn merged commit 1b16f4d into master Sep 11, 2018
@Pomax Pomax deleted the pni branch October 30, 2018 22:14
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.

None yet

3 participants