Skip to content

Conversation

@alkhasov
Copy link
Contributor

@alkhasov alkhasov commented Mar 4, 2019

Current:
image

With PR:
image

En original:
image

@alkhasov alkhasov changed the title Fixed overlapping text in header Fixing header, search bar no more overlaps link Mar 4, 2019
@netlify
Copy link

netlify bot commented Mar 4, 2019

Deploy preview for ru-reactjs ready!

Built with commit 645843e

https://deploy-preview-231--ru-reactjs.netlify.com

@lex111
Copy link
Member

lex111 commented Mar 4, 2019

Спасибо, про "лишний" пункт не знал.

Это, конечно, хорошо, но пока ждём решение на самом оф. сайте, поскольку это появилось после появления пункта с языками (вроде как здесь решается reactjs/react.dev#1751)

Хотя в качестве временного решения можем принять? сс @another-guy я не уверен на этот счёт.

@lex111 lex111 requested a review from another-guy March 4, 2019 23:29
@alkhasov
Copy link
Contributor Author

alkhasov commented Mar 4, 2019

Оу, а я не знал про тот PR.

Думаю хороший вариант сделать менюшку полноценную ниже уровнем, в виде списка чтобы наверняка все помещалось и которая схлопывается в бургер после скролла.

@lex111
Copy link
Member

lex111 commented Mar 4, 2019

Неизвестно, что там дальше будет, обычно это не быстро, к сожалению. Поэтому склоняюсь к тому, чтобы принять данное решение, в принципе почему бы и нет.

@alkhasov
Copy link
Contributor Author

alkhasov commented Mar 5, 2019

Тогда можно локально подумать и над вариантом вроде этого reactjs/react.dev#1751 (comment)

@lex111
Copy link
Member

lex111 commented Mar 5, 2019

@musewick классно выглядит, но давайте ограничимся самым минимумом, либо уже решать в том же PR.

@lex111
Copy link
Member

lex111 commented Mar 5, 2019

Вообще есть задача сделать выпадающий список, может быть тогда надпись даже уберут, как на сайте вебпака сделано.

@alkhasov
Copy link
Contributor Author

alkhasov commented Mar 5, 2019

@musewick классно выглядит, но давайте ограничимся самым минимумом, либо уже решать в том же PR.

Хорошо.

Вообще есть задача сделать выпадающий список, может быть тогда надпись даже уберут, как на сайте вебпака сделано.

У вебпака отличный вариант, нравится что можно сразу в детали перейти.

Еще на самом деле меню должно нынче снизу появляться на мобильниках, с распространением длинных экрананов. Меню может скроллится вверх и стать полноценной индекс страницей. Завтра нарисую такой вариант, он кажется лучше по многим аспектам.

@lex111
Copy link
Member

lex111 commented Mar 5, 2019

@musewick это здорово, но не факт, что его примут, но попробовать, конечно, стоит 👍

@alkhasov
Copy link
Contributor Author

alkhasov commented Mar 5, 2019

Ну я надеюсь, что напишут фидбек хотя бы, отстой, не отстой, все остальное это дополнительно :D

@another-guy
Copy link
Collaborator

@lex111 @musewick прошу прощения, у меня нет сложившегося мнения.

В плане UX/UI, однозначно, решение мне нравится. А вот применять ли его прямо сейчас в ru.react.org или идти через родительский репозиторий, я не советчик. Это классический эффект второй системы. Если мы пойдём этим путём, то придётся поддерживать "форк". Лично у меня не хватит компетенции в CSS, чтобы решать проблемы. 🤷‍♂️

Полностью доверяю вашему решению. Может @gcor, @Heegiiny, @ntishkevich могут что-то более осмысленное подсказать?

@another-guy another-guy removed their assignment Mar 6, 2019
@another-guy another-guy removed their request for review March 6, 2019 05:53
@another-guy another-guy added the needs review A pull request ready to be reviewed label Mar 6, 2019
Copy link
Member

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

Если мы пойдём этим путём, то придётся поддерживать "форк". Лично у меня не хватит компетенции в CSS, чтобы решать проблемы

Я думаю, ты преувеличиваешь, тут всего лишь ширина блока, мелкое изменение, хотя не означает, что оно может быть безвредным, но тем не менее, в данном случае это только на пользу.

@lex111 lex111 merged commit 3e93eaf into reactjs:master Mar 10, 2019
@lex111
Copy link
Member

lex111 commented Mar 10, 2019

@musewick спасибо!

@another-guy another-guy removed the needs review A pull request ready to be reviewed label Mar 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request under discussion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants