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

Добавляет доку про window.close() #5527

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

ra1nbow1
Copy link
Member

@ra1nbow1 ra1nbow1 commented Oct 13, 2024

Описание

Closes #315.

Чек-лист

  • Текст оформлен согласно руководству по стилю
  • Ссылки на внутренние материалы начинаются со слеша и заканчиваются слэшем либо якорем на заголовок (/css/color/, /tools/json/, /tools/gulp/#kak-ponyat)
  • Ссылки на картинки, видео и демки относительные (images/example.png, demos/example/, ../demos/example/)

@github-actions github-actions bot added js Контент по JavaScript дока Справочный материал labels Oct 13, 2024
js/window-close/index.md Outdated Show resolved Hide resolved
Copy link
Member

@HellSquirrel HellSquirrel left a comment

Choose a reason for hiding this comment

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

Класс. Спасибо. Давай чуть подправим код и помержим

js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
@TatianaFokina
Copy link
Member

@ra1nbow1, посмотришь на комменты от Полины?

@ra1nbow1 ra1nbow1 requested a review from Inventoris as a code owner January 23, 2025 06:12
Copy link
Member

@Inventoris Inventoris left a comment

Choose a reason for hiding this comment

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

Привет! Почитал, посмотрел, оставил комментов ниже 🙌

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

js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
@vitya-ne vitya-ne requested a review from skorobaeus as a code owner January 25, 2025 20:00
@vitya-ne
Copy link
Contributor

@HellSquirrel,@Inventoris я тут радикально "похозяйничал" с согласия автора, поглядите.
Я добавил демо для примера. И убрал упоминие про 'top-level window'. Я видел что оно есть на MDN, но так как нет конкретного примера и объяснения, то мне кажется лучше это не упоминать.

Copy link
Member

@Inventoris Inventoris left a comment

Choose a reason for hiding this comment

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

Привет, огонь что забрал. Чуть добавил комментов:

js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
js/window-close/index.md Outdated Show resolved Hide resolved
Copy link
Member

@Inventoris Inventoris left a comment

Choose a reason for hiding this comment

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

Добавил ещё пару предложений :)

P.S. И про 'top-level window' тут думаю. Всё таки штука такая есть, я про это:

...or on top-level windows that have a single history entry.

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


## Пример

```js
let openedWindow
// Создаём функцию для открытия окна с Докой
const openWindow = () => window.open('https://doka.guide/index.html')
Copy link
Member

Choose a reason for hiding this comment

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

Для чего нам нужен путь? Может оставим версию без него, потому что откроется тоже самое:

Suggested change
const openWindow = () => window.open('https://doka.guide/index.html')
const openWindow = () => window.open('https://doka.guide/')

Comment on lines +80 to +89
const closeWindow = (windowProxy) => windowProxy.close()

let currentWindow

buttonOpen.addEventListener('click', function (event) {
buttonOpen.disabled = true
buttonClose.disabled = false

currentWindow = openWindow()
})
Copy link
Member

Choose a reason for hiding this comment

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

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

Suggested change
const closeWindow = (windowProxy) => windowProxy.close()
let currentWindow
buttonOpen.addEventListener('click', function (event) {
buttonOpen.disabled = true
buttonClose.disabled = false
currentWindow = openWindow()
})
const closeWindow = (windowProxy) => windowProxy.close()
let currentWindow
const watchTheWindowClose = () => {
const timer = setInterval(function() {
if (currentWindow.closed) {
clearInterval(timer)
buttonClose.disabled = true
buttonOpen.disabled = false
}
}, 100)
}
buttonOpen.addEventListener('click', function (event) {
buttonOpen.disabled = true
buttonClose.disabled = false
currentWindow = openWindow()
watchTheWindowClose()
})

Copy link
Contributor

Choose a reason for hiding this comment

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

Мне кажется, что это лишнее. Статья описывает закрытие окна, зачем же нам демонстрировать слежение за ним?

@vitya-ne
Copy link
Contributor

Добавил ещё пару предложений :)

P.S. И про 'top-level window' тут думаю. Всё таки штука такая есть, я про это:

...or on top-level windows that have a single history entry.

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

Упомянуть что ?

Copy link

Превью контента из 167fa15 опубликовано.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
js Контент по JavaScript дока Справочный материал
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Интерфейс командной строки
6 participants