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

Add meta title for about page #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add meta title for about page #108

wants to merge 1 commit into from

Conversation

lex111
Copy link
Member

@lex111 lex111 commented Jan 6, 2017

Сейчас нет собственного заголовка у страницы "О нас", поэтому может стоит его добавить, но исходя из контента страницы, лучше подошло бы название "Пожертвования", но так в меню страница представлена как "О нас", то и её мета-заголовок такой же, чтобы не нарушать единообразие.

@@ -1,3 +1,5 @@
- title t("pages.about.link")
Copy link
Member

Choose a reason for hiding this comment

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

Тут можно использовать лейзи матчинг: title('.link'). И ключ логичнее назвать title :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Не добавлялся специально новый ключ, исходя из того что название пункта меню = названию страницы, а есть ли сейчас в этом смысл, т.к. будут два ключа с одинаковым значением?

Copy link
Member

@kovalevsky kovalevsky Jan 28, 2017

Choose a reason for hiding this comment

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

Сейчас есть ключи и link, и title.
link логичнее было бы назвать menu_link_title (ок, не относится к pr, но я бы переименовал, т.к. это пока единственный пример такого нейминга и он не очень).
Что использовать в качестве title – похоже вопрос открытый, @vitallium есть мысли на этот счет?

И вне зависимости от имени ключа – стоит использовать лейзи.

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