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

BEMHTML: use proper boolean values for attribute #1747

Merged
merged 1 commit into from
Feb 11, 2016
Merged

BEMHTML: use proper boolean values for attribute #1747

merged 1 commit into from
Feb 11, 2016

Conversation

gela-d
Copy link
Member

@gela-d gela-d commented Feb 10, 2016

@deeonis deeonis added the ready label Feb 10, 2016
@voischev
Copy link
Member

А где почитать — почему теперь булеан нужно строку оборачивать?

@tadatuta
Copy link
Member

@voischev На проектах — не нужно. Этот коммит про то, чтобы один набор эталонов подходил для bem-xjst 4.x и bem-xjst 5.x одновременно.

В следующем мажоре эти изменения нужно будет откатить.

@@ -1,3 +1,3 @@
block('button').mod('togglable', 'radio').attrs()(function() {
return this.extend(applyNext(), { 'aria-pressed' : !!this.mods.checked });
return this.extend(applyNext(), { 'aria-pressed' : (!!this.mods.checked).toString() });
Copy link
Member

Choose a reason for hiding this comment

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

String(!!this.mods.checked) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Спасибо, везде поправила.

@voischev
Copy link
Member

@tadatuta понял спасибо

@qfox
Copy link
Member

qfox commented Feb 10, 2016

В следующем мажоре эти изменения нужно будет откатить.

@tadatuta Разве нужно будет?

@@ -17,8 +17,8 @@ block('menu-item')(
attrs = {
role : role,
id : this.generateId(),
'aria-disabled' : mods.disabled,
'aria-checked' : menuMode && !!mods.checked
'aria-disabled' : mods && mods.disabled && 'true' || undefined,
Copy link
Member

Choose a reason for hiding this comment

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

наличие this.mods гарантируется ядром, а если mods.disabled окажется falsy, то это и есть undefined так что можно сократить до

'aria-disabled' : mods.disabled && 'true'

в остальном — lgtm

@gela-d
Copy link
Member Author

gela-d commented Feb 11, 2016

@tadatuta все тесты пропассились, кроме диста https://travis-ci.org/bem/bem-components/jobs/108494214 Но я не поняла что он хочет, если честно.

@sipayRT
Copy link
Contributor

sipayRT commented Feb 11, 2016

он просит, чтобы его перезапустили)

@sipayRT
Copy link
Contributor

sipayRT commented Feb 11, 2016

@gela-d теперь все ок

@gela-d
Copy link
Member Author

gela-d commented Feb 11, 2016

он просит, чтобы его перезапустили)
Я перезапускала, он и второй раз валился почему-то.
спасибо!

gela-d pushed a commit that referenced this pull request Feb 11, 2016
BEMHTML: use proper boolean values for attribute
@gela-d gela-d merged commit 1a4463f into v2 Feb 11, 2016
@gela-d gela-d deleted the issues/1745@v2 branch February 11, 2016 10:50
@deeonis deeonis removed the ready label Feb 11, 2016
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.

None yet

6 participants