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

Class does not remove when value becomes false #2082

Closed
syuilo opened this Issue Nov 23, 2016 · 3 comments

Comments

Projects
None yet
6 participants
@syuilo
Contributor

syuilo commented Nov 23, 2016

Help us to manage our issues by answering the following:

Describe your issue:

Class does not remove when value becomes false.

I think that behavior is against the explanation: A shorthand syntax for class names is available: class={ completed: done } renders to class="completed"when the value of done is a true value..

This bug(?) only encounter on v3.0.0.

For details, please refer to the examples below.

Can you reproduce the issue?

Yes, Compare behaviors:

3.0.0:
http://jsfiddle.net/wf7bkvur/128/

2.x:
http://jsfiddle.net/wf7bkvur/127/

Which version of Riot does it affect?

v3.0.0

How would you tag this issue?

  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance

Thanks, Great lib! ❤️ cheers 🍺

@syuilo syuilo changed the title from Class does not update when value becomes false to Class does not remove when value becomes false Nov 23, 2016

@YoussefKababe

This comment has been minimized.

Show comment
Hide comment
@YoussefKababe

YoussefKababe Nov 23, 2016

Contributor

I confirm! Also doing something like this won't remove the class when it's false:

class={ done ? 'completed' : '' }

I ended up doing this while waiting for it to be fixed, and it works:

class={ done ? 'completed' : 'incomplete' }
Contributor

YoussefKababe commented Nov 23, 2016

I confirm! Also doing something like this won't remove the class when it's false:

class={ done ? 'completed' : '' }

I ended up doing this while waiting for it to be fixed, and it works:

class={ done ? 'completed' : 'incomplete' }

@cognitom cognitom added the bug label Nov 23, 2016

@fabien

This comment has been minimized.

Show comment
Hide comment
@fabien

fabien Nov 23, 2016

Contributor

Here's another 'workaround':

<p class={ active: isActive, foo: true }>Riot{ isActive ? '.js' : '' }</p>

Note the additional attribute - if I set foo: false it stops working again. Definitely a bug.

Another 'workaround' which might help uncover the cause if this:

<p class=" { active: isActive }">Riot{ isActive ? '.js' : '' }</p> (a space inserted)

Contributor

fabien commented Nov 23, 2016

Here's another 'workaround':

<p class={ active: isActive, foo: true }>Riot{ isActive ? '.js' : '' }</p>

Note the additional attribute - if I set foo: false it stops working again. Definitely a bug.

Another 'workaround' which might help uncover the cause if this:

<p class=" { active: isActive }">Riot{ isActive ? '.js' : '' }</p> (a space inserted)

@syuilo syuilo referenced this issue Nov 23, 2016

Closed

Migrate to Riot 3.0.0 #79

1 of 3 tasks complete

@cognitom cognitom referenced this issue Nov 25, 2016

Closed

Official Todo App on plunker have errors #2093

0 of 1 task complete
@believer-ufa

This comment has been minimized.

Show comment
Hide comment
@believer-ufa

believer-ufa Nov 26, 2016

Hello. I think we need to have all keys from expression in update.js component to easy update DOM element classes. Because now you have this code:

if (value === 0 || value && typeof value !== T_OBJECT)
  setAttr(dom, attrName, value)

If value is empty, Riot.js will dont delete exists classes.

If think you must add after it something like this:

if (attrName === 'class') {
  // Add or remove classes, based on current expr keys and final value
}

Maybe, through classList api? Or through something like this methods?

You already have values. Main problem now is get all keys from expression. Maybe create in riot-tmpl something like getKeys method? And use it in this place?


Or just add remAttr function call if value is empty?

if (value === 0 || value && typeof value !== T_OBJECT)
  setAttr(dom, attrName, value)
else
  remAttr(dom, attrName)

This approach fixes this issue too.

believer-ufa commented Nov 26, 2016

Hello. I think we need to have all keys from expression in update.js component to easy update DOM element classes. Because now you have this code:

if (value === 0 || value && typeof value !== T_OBJECT)
  setAttr(dom, attrName, value)

If value is empty, Riot.js will dont delete exists classes.

If think you must add after it something like this:

if (attrName === 'class') {
  // Add or remove classes, based on current expr keys and final value
}

Maybe, through classList api? Or through something like this methods?

You already have values. Main problem now is get all keys from expression. Maybe create in riot-tmpl something like getKeys method? And use it in this place?


Or just add remAttr function call if value is empty?

if (value === 0 || value && typeof value !== T_OBJECT)
  setAttr(dom, attrName, value)
else
  remAttr(dom, attrName)

This approach fixes this issue too.

believer-ufa added a commit to believer-ufa/riot that referenced this issue Nov 26, 2016

Try fo fix #2082
Trying to fix #2082 issue about classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment