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 notification for old browsers #475

Merged
merged 5 commits into from
Jun 15, 2018
Merged

Add notification for old browsers #475

merged 5 commits into from
Jun 15, 2018

Conversation

Trufi
Copy link
Contributor

@Trufi Trufi commented Jun 5, 2018

В рамках #474 добавил показ сообщения о том, что браузер пользователя не поддерживается.

Чтобы сообщение не показывалось, нужно передать в карту опцию при инициализации museum: false.

@Trufi
Copy link
Contributor Author

Trufi commented Jun 5, 2018

Тут я подумал, что название museum, возможно, не самое удачное :)
Но не знаю на что поменять, кроме опции в карте, оно должно прослеживаться в названии модуля, класса и всех связанных файлов.

@@ -0,0 +1,83 @@
var ie9 = (function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Не стоит ли эту проверку в DG.Browser и вынести?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

ОК

@osvodef
Copy link
Contributor

osvodef commented Jun 6, 2018

Лично я не против названия museum))

<td><code><b>museum</b></code></td>
<td><code>Boolean</code></td>
<td><code>true</code></td>
<td>Whether the message about unsupported browser be displayed.</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

be -> will be ?
@RLRR

Copy link
Contributor

Choose a reason for hiding this comment

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

Да, точняк, надо поправить

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Добавил

@Trufi
Copy link
Contributor Author

Trufi commented Jun 14, 2018

Поднял значение z-index для музея, чтобы он показывался поверх контролов.
Добавил использования localStorage, чтобы не показывать музей повторно для тех, кто его уже скрывал.

@Trufi Trufi requested a review from osvodef June 14, 2018 04:54
@@ -70,6 +79,14 @@ DG.Map.Museum = DG.Handler.extend({

_onCloseButtonClick: function(e) {
DG.DomEvent.stop(e);

// Old Safari throws error when localStorage.getItem calls in private mode
Copy link
Contributor

Choose a reason for hiding this comment

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

calls → is called

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Поправил

@vkorostelev vkorostelev merged commit b616da0 into master Jun 15, 2018
@vkorostelev vkorostelev deleted the museum branch June 15, 2018 10:15
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.

4 participants