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

dropdown: get rid of wrapping root div node (close #1392) #1393

Merged
merged 1 commit into from
Aug 14, 2015
Merged

Conversation

aristov
Copy link
Contributor

@aristov aristov commented Mar 6, 2015

Resolves #1392

@aristov aristov force-pushed the issues/1392@v2 branch 3 times, most recently from 0139e5a to adb4bcc Compare March 13, 2015 12:40
@tadatuta
Copy link
Member

why not to generate id automatically by default?

@aristov
Copy link
Contributor Author

aristov commented Mar 18, 2015

@tadatuta I generate id automatically in template, but if I don't set id manually in tmpl-specs, then I get difference in data-bem attribute, because templaters generate a new id each time.

@tadatuta
Copy link
Member

👍

@aristov
Copy link
Contributor Author

aristov commented Mar 18, 2015

@tadatuta maybe we can consider this case in html-differ and ignore difference for id param in data-bem attribute.

@tadatuta
Copy link
Member

@aristov not sure if it's possible as we need to check that id is the same for two corresponding DOM nodes.

@narqo
Copy link
Member

narqo commented Mar 19, 2015

  1. use elem instead of custom modes in BEMHTML as it was before
  2. add tests for proper destruct of popup

js()(true),
def()(function() {
this.ctx.js = this.extend({ id : this.generateId() }, this.ctx.js);
applyCtx([apply('switcher'), apply('popup')]);
Copy link
Member

Choose a reason for hiding this comment

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

applyCtx([
  {
    elem : 'switcher',
    js : this.extend(apply('js'), this.ctx.js),
    ···
  },
  { elem : 'popup' }
])

@aristov aristov changed the title dropdown: get rid of wrapping root div node (close #1392) [WIP] dropdown: get rid of wrapping root div node (close #1392) Mar 19, 2015
@aristov aristov changed the title [WIP] dropdown: get rid of wrapping root div node (close #1392) dropdown: get rid of wrapping root div node (close #1392) Apr 6, 2015
@aristov
Copy link
Contributor Author

aristov commented May 14, 2015

TODO: ensure that dropdowns mixes work

@aristov aristov changed the title dropdown: get rid of wrapping root div node (close #1392) [WIP] dropdown: get rid of wrapping root div node (close #1392) May 14, 2015
@aristov aristov changed the title [WIP] dropdown: get rid of wrapping root div node (close #1392) dropdown: get rid of wrapping root div node (close #1392) Jun 10, 2015
@aristov
Copy link
Contributor Author

aristov commented Jun 11, 2015

TODO: ensure that popup's and dropdown__switcher's mixes work

@aristov
Copy link
Contributor Author

aristov commented Jul 23, 2015

For me: add return before apply*, squash and merge.

aristov pushed a commit that referenced this pull request Aug 14, 2015
dropdown: get rid of wrapping root `div` node (close #1392)
@aristov aristov merged commit 085fd81 into v2 Aug 14, 2015
@aristov aristov deleted the issues/1392@v2 branch August 14, 2015 18:28
@narqo narqo mentioned this pull request Oct 20, 2015
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

4 participants