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

[menu] move specific template logic which deal with menu-item's checked mod from menu to common menu_mode #1520

Closed
wants to merge 1 commit into from

Conversation

Guria
Copy link
Contributor

@Guria Guria commented May 28, 2015

iterate through content only within all menu_mode_*
do it in content mode instead of def
use applyNext instead of setting global ctx properties

resolves #1513 #1516

@Guria Guria changed the title [menu] move specific template logic which deal with menu-item's checed mod from menu to common menu_mode [menu] move specific template logic which deal with menu-item's checked mod from menu to common menu_mode May 28, 2015
@Guria
Copy link
Contributor Author

Guria commented May 28, 2015

Why js tests could fail if all templates tests are ok? JS wasn't touched in commit.

@Guria
Copy link
Contributor Author

Guria commented May 28, 2015

It seems that spec tests doesn't include menu_mode.bemhtml into test bundle while tmpl-specs works as expected.

@@ -0,0 +1,40 @@
block('menu')
.match(function(){
return this.ctx.mods['mode'];
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be like this:

return typeof this.mods.mode !== 'undefined';

@Guria
Copy link
Contributor Author

Guria commented May 28, 2015

cc @tadatuta @blond please help
I have no enough experience to understand why menu_mode doesn't included in specs tests

…ed mod from menu to common menu_mode

iterate through content only within all menu_mode_*
do it in content mode instead of def
use applyNext instead of setting global ctx properties
@Guria
Copy link
Contributor Author

Guria commented May 28, 2015

@tadatuta I have resolved issue with specs tests.
Hope this pr will be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants