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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Q: null as class name #1764

Closed
simov opened this Issue Mar 31, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@simov
Copy link

simov commented Mar 31, 2017

馃憢

I've been using this construct in Mithril since 0.2.5:

var something = false
m.mount(document.querySelector('body'), {
  view: () =>
    m('.hey', {class: something ? 'other' : null})
})
// at some point
m.redraw()

In 1.0.1 this results in:

<div class="hey null"></div>

But in 1.1.0 the result is:

<div class="null"></div>

So I tried with undefined:

var something = false
m.mount(document.querySelector('body'), {
  view: () =>
    m('.hey', {class: something ? 'other' : undefined})
})
// at some point
m.redraw()

And it worked:

<div class="hey"></div>

Is that a bug or a feature?

@pygy

This comment has been minimized.

Copy link
Member

pygy commented Mar 31, 2017

I can't repro this: <div class="null"></div>.

I get <div class="hey"></div> in both cases...

http://jsbin.com/tipuhuyupa/edit?css,js,output

@simov

This comment has been minimized.

Copy link

simov commented Mar 31, 2017

Try this:

m.mount(document.body, {
  view: function () {
    return [
      m('.hey', {class: undefined}, 'ho'),
      m('.hey', {class: null}, 'ho')
    ]
  }
})
m.redraw()

The combination is mount + redraw

@pygy

This comment has been minimized.

Copy link
Member

pygy commented Mar 31, 2017

The plot thickens...

The problems comes from the way class is normalized to className, but class is left as either null or undefined on the attrs. When it is null, it is problematic on update.

Previously, a null class was treated as existing and concatenated to .hey.

@isiahmeadows shouldn't we delete attrs.class or set it to always be undefined?

@pygy

This comment has been minimized.

Copy link
Member

pygy commented Apr 3, 2017

Fixed in #1769, v1.1.1 will be out soon

pygy added a commit to pygy/mithril.js that referenced this issue Apr 3, 2017

pygy added a commit to pygy/mithril.js that referenced this issue Apr 4, 2017

@pygy pygy closed this in 98e3cbd Apr 4, 2017

pygy added a commit that referenced this issue Apr 4, 2017

Merge pull request #1769 from pygy/fix-class-null
hyperscript: Revert attrs.class creation logic to what we had in v1.0.1. fix #1764
@pygy

This comment has been minimized.

Copy link
Member

pygy commented Apr 4, 2017

Fixed in v1.1.1 (back to what the v1.0.1 was doing, for now). See #1773 for future discussion.

@pygy

This comment has been minimized.

Copy link
Member

pygy commented Apr 4, 2017

@simov BTW, If I were you I'd use "" instead of null, it will behave better (and what happens won't change with future fixes to the class situation).

@simov

This comment has been minimized.

Copy link

simov commented Apr 4, 2017

I'm pretty sure it wasn't working as expected with empty string at some point (version) ... though I can't remember exactly. I'll try it out again.

@pygy

This comment has been minimized.

Copy link
Member

pygy commented Apr 4, 2017

AFAICT the only "problem" now with "" is that it adds an extra space if used in conjunction with a class selector.

m(".x", {class: ""}).attrs is {class: undefined, className: "x "}.

The presence of class: undefined is problematic but not specific to ""

http://jsbin.com/roqajexobu/1/edit?html,js,console

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment