Skip to content
This repository was archived by the owner on Mar 9, 2021. It is now read-only.

Single quotes#46

Open
Alge wants to merge 4 commits into
masterfrom
singleQuotes
Open

Single quotes#46
Alge wants to merge 4 commits into
masterfrom
singleQuotes

Conversation

@Alge

@Alge Alge commented May 6, 2018

Copy link
Copy Markdown
Collaborator

No description provided.

@Alge Alge requested review from Victorystick and reinefjord May 6, 2018 13:45
@@ -0,0 +1,39 @@
<style>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel like <style></style> should be in the template, the .css file should only contain CSS.

display: none;
}
</style>
{% include 'strecklista/components/quote.css' %}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

<style>
{% include 'strecklista/components/quote.css' %}
</style>

<div class="detail">{{ quote.timestamp }}</div>
</div>
<div class="quote-link">
Permanent länk: <a href="{{ request.get_host }}/quote/{{ quote.id }}">{{ request.get_host }}/quote/{{ quote.id }}</a>

@reinefjord reinefjord May 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Anchor must have the protocol in href, otherwise request.get_host is interpreted as the protocol. Should be href="//{{ request.get_host C}}[...]", note the //. Isn't {% url 'view.name' quote=quote.id %} the correct way to do this though?

Also, <a href="...">Permanent länk</a> is IMHO better UX. It's how hyperlinks are intended to be used.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants