Skip to content

i-bem: use resetApplyNext before entering content #899

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.

Sign up for GitHub

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

Jump to bottom
Merged
aristov merged 2 commits into from
Jul 9, 2015
Merged

i-bem: use resetApplyNext before entering content #899

aristov merged 2 commits into from
Jul 9, 2015

Conversation

@indutny
Copy link
Contributor

@indutny indutny commented Mar 5, 2015

Skip all applyNext() flags before entering the content to support
recursive blocks with applyNext() (see test).

Depends on: veged/xjst#41

@indutny
Copy link
Contributor Author

indutny commented Mar 5, 2015

cc @veged, this is what you has dreamt about :D

@qfox
Copy link
Member

qfox commented Mar 5, 2015

👍 :-D

@veged veged added the ready label Apr 29, 2015
@indutny indutny changed the title i-bem: use skipApplyNext before entering content i-bem: use resetApplyNext before entering content May 6, 2015
@indutny
Copy link
Contributor Author

indutny commented May 6, 2015

Turns out skipApplyNext is not required to accomplish this. The only problem with resetApplyNext is that it breaks the page block, and this block in fact needs to be fixed anyway.

@indutny indutny force-pushed the feature/skip-apply-next-in-content branch from 5dce5f1 to edfc122 Compare May 6, 2015 16:02
@indutny
Copy link
Contributor Author

indutny commented May 6, 2015

Updated PR, please re-review. ;)

veged
veged reviewed May 7, 2015
View reviewed changes
common.blocks/i-bem/i-bem.test.bemhtml/files/i-bem/skip-apply-next.bemhtml Outdated
Copy link
Member

@veged veged May 7, 2015

Choose a reason for hiding this comment

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

why not just applyNext({ 'ctx.tag': 'span' }) without local?

@indutny
Copy link
Contributor Author

indutny commented May 10, 2015

@veged: done

@veged
Copy link
Member

veged commented May 12, 2015

we also need to do same in https://github.com/bem/bem-core/blob/feature/skip-apply-next-in-content/common.blocks/i-bem/i-bem.bemtree and separate pull request for v3 branch

@aristov
Copy link
Contributor

aristov commented May 14, 2015

@indutny ping

@aristov
Copy link
Contributor

aristov commented May 14, 2015

@indutny also we need the same in bem-bl.

@veged veged mentioned this pull request May 25, 2015
Closed
@apsavin
Copy link
Contributor

apsavin commented Jun 18, 2015

Guys?..
When it will be merged? Do you have any estimations?

@tadatuta
Copy link
Member

tadatuta commented Jun 26, 2015

@indutny ping?

@apsavin
Copy link
Contributor

apsavin commented Jun 30, 2015

@veged ?.. @indutny ?..

@indutny
Copy link
Contributor Author

indutny commented Jul 2, 2015

Going to land it everywhere soon ;) Sorry for the waiting.

@indutny
Copy link
Contributor Author

indutny commented Jul 5, 2015

@veged pushed commit for bemtree

@indutny
Copy link
Contributor Author

indutny commented Jul 5, 2015

PR for v3: #1085

@indutny
i-bem: use resetApplyNext before entering content
b1dc50c 
Skip all `applyNext()` flags before entering the `content` to support
recursive blocks with `applyNext()` (see test).
@indutny indutny force-pushed the feature/skip-apply-next-in-content branch from 6460deb to b1dc50c Compare July 5, 2015 23:21
indutny added a commit to veged/xjst that referenced this pull request Jul 6, 2015
@indutny
compiler: resetApplyNext()
3cc9749 
See: bem/bem-core#899
@indutny
Copy link
Contributor Author

indutny commented Jul 6, 2015

Old xjst support: veged/xjst@3cc9749

@indutny
Copy link
Contributor Author

indutny commented Jul 6, 2015

Ooops, decided to do it in 0.10 branch instead.

indutny added a commit to veged/xjst that referenced this pull request Jul 6, 2015
@indutny
compiler: resetApplyNext()
3169083 
See: bem/bem-core#899
@indutny
Copy link
Contributor Author

indutny commented Jul 6, 2015

Here it goes: veged/xjst@3169083

@indutny indutny mentioned this pull request Jul 6, 2015
Open
@indutny
Copy link
Contributor Author

indutny commented Jul 6, 2015

Here goes the bem-bl patch: bem/bem-bl#653 . Sorry for a looong delay, it must have been very frustrating for you.

aristov pushed a commit that referenced this pull request Jul 9, 2015
Merge pull request #899 from bem/feature/skip-apply-next-in-content
3c2836f 
i-bem: use resetApplyNext before entering `content`
@aristov aristov merged commit 3c2836f into v2 Jul 9, 2015
@aristov aristov removed the ready label Jul 9, 2015
@aristov aristov deleted the feature/skip-apply-next-in-content branch July 9, 2015 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@veged veged

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@indutny @qfox @veged @aristov @apsavin @tadatuta