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

checkboxes & radios not updating when underlying data changes #40

Open
wildhart opened this issue Jun 15, 2016 · 10 comments
Open

checkboxes & radios not updating when underlying data changes #40

wildhart opened this issue Jun 15, 2016 · 10 comments
Assignees

Comments

@wildhart
Copy link

Say you have an MDL checkbox:

<label class="mdl-checkbox mdl-js-checkbox mdl-js-ripple-effect" for="checkbox-1">
  <input type="checkbox" id="checkbox-1" class="mdl-checkbox__input" checked={{isChecked}}>
  <span class="mdl-checkbox__label">Checkbox</span>
</label>

If the isChecked value is changed by another client then the MDL checkbox does not update. The checked property of the hidden <input> element changes, but this does not get converted into toggling the 'is-checked' class of the <label> element.

My work-around is to add the 'is-checked' class myself using:

<label class="mdl-checkbox mdl-js-checkbox mdl-js-ripple-effect {{#if isChecked}}is-checked{{/if}}" for="checkbox-1">
  <input type="checkbox" id="checkbox-1" class="mdl-checkbox__input" checked={{isChecked}}>
  <span class="mdl-checkbox__label">Checkbox</span>
</label>

This seems to work perfectly, but it would be nice if this was handled automatically.

I know that this is not a bug of meteor-mdl, rather it is a bug in MDL itself. See MDL bug #1504, also 4164/1757. I've attached a screenshot here. Most relevant comment there is:

This isn't feasible without observers. If a developer changes the checked property using JS, no event is fired. It needs to be manually ran by developers to have any event handlers fire off. This is why we have the internal API for handling these things and updating the presentation.

Since they are expecting this to be handled by developers, I think these observers should be incorporated into meteor-mdl.

Thanks for the package though!

@Zodiase
Copy link
Owner

Zodiase commented Jun 15, 2016

I'm also mostly coping with the same as your work-around there. This is a good idea though. I'll see what I can do.

@Zodiase Zodiase self-assigned this Jun 15, 2016
@Zodiase
Copy link
Owner

Zodiase commented Jun 15, 2016

Also you are more than welcome to make a PR for this functionality.

@wildhart
Copy link
Author

It looks like MutationObserver doesn't fire for property change events. The best I can come up with is

setInterval(function() {
    $("input.mdl-checkbox__input, input.mdl-radio__button").each(function() {
        this.id && $("label[for="+this.id+"]").toggleClass('is-checked',this.checked);
    });
}, 1000);

Cross-referencing the input's id with the label's "for=" property means that the input doesn't have to be a child of the label. Still doesn't feel very elegant though.

Where to put this code within meteor-mdl to be the most efficient (only executed once, or only executed if there are MDL radios or checkboxes on the page) is well beyond me.

@Zodiase
Copy link
Owner

Zodiase commented Jun 16, 2016

It's a shame there isn't any way of monitoring property changes. Polling should be the last resort and is really a bad practice.

I have a slightly awkward solution. Since MutationObserver can observe attribute changes, and as far as I know it's the most efficient solution for reacting to changes, what if I put the Blaze reactive content in an attribute like this:

<label
  class="mdl-checkbox mdl-js-checkbox mdl-js-ripple-effect"
  for="checkbox-1"
  data-mdl-checkbox-checked="{{isChecked}}">
  <input type="checkbox" id="checkbox-1" class="mdl-checkbox__input">
  <span class="mdl-checkbox__label">Checkbox</span>
</label>

With that now I can monitor the changes! And there's only one field to code and for Blaze to change. I put the attribute on the label element because MDL registers the component object on there. Also the super-long naming is to avoid interference with other logic and I think it helps narrow down what each MutationObserver needs to do and could be more efficient.

What do you think?

@wildhart
Copy link
Author

That would work, but it's not obvious and not automatic - the developer would need to read your README to know to use the data-mdl-checkbox-checked attribute, in which case it's just as easy to recommend the {{#if isChecked}}is-checked{{/if}} class workaround, and then no observers required in your code.

@Zodiase
Copy link
Owner

Zodiase commented Jun 17, 2016

That's true.

What if I provide wrapped component templates like:

{{> MDLCheckbox id="foo" className="more_classes to_add" checked=someHelper}}

I used to not like this idea as I think it unnecessarily tries to over-simplify problems and would perhaps make things harder to use.

@matteolavaggi
Copy link

Hi @Zodiase i've a problem using mdl checkbox: I've implemented the "double" control for the checked property and use vents to handle and store a mongo state for the checked value of a list. My problem is that when the template is rendered , the default label class "is-checked" is inserted in the cehckbox label! So how is the right way to interact with your component?

Using this workaround its not a good job, cause i can control the is-checked class of label and the checked attribute of the input element, but i can not prevent the component to renderd defautl with their "is-checked" class, so if i need to store the is-checked property, every time i load the template, i get all the checkbox checked, also if the #if blaze block is false. Thanks
<label class="mdl-checkbox mdl-js-checkbox mdl-js-ripple-effect {{#if isChecked}}is-checked{{/if}}" for="checkbox-1"> <input type="checkbox" id="checkbox-1" class="mdl-checkbox__input" checked={{isChecked}}> <span class="mdl-checkbox__label">Checkbox</span> </label>

@Zodiase
Copy link
Owner

Zodiase commented Jan 17, 2017

@matteolavaggi I could not reproduce the issue as you described.

Here's my app files:

/client/main.html

<head>
  <title>foobar</title>
</head>

<body>
  <h1>Welcome to Meteor!</h1>

  {{> hello}}
</body>

<template name="hello">
  <label class="mdl-checkbox mdl-js-checkbox mdl-js-ripple-effect {{#if isChecked}}is-checked{{/if}}" for="checkbox-1">
    <input type="checkbox" id="checkbox-1" class="mdl-checkbox__input" checked={{isChecked}}>
    <span class="mdl-checkbox__label">Checkbox (ReactiveVar)</span>
  </label>
  
  <label class="mdl-checkbox mdl-js-checkbox mdl-js-ripple-effect {{#if inCollection}}is-checked{{/if}}" for="checkbox-2">
    <input type="checkbox" id="checkbox-2" class="mdl-checkbox__input" checked={{inCollection}}>
    <span class="mdl-checkbox__label">Checkbox (Collection)</span>
  </label>
</template>

/client/main.js

import { Template } from 'meteor/templating';
import { ReactiveVar } from 'meteor/reactive-var';
import { Mongo } from 'meteor/mongo';

import './main.html';

Template.hello.onCreated(function helloOnCreated() {
  this.checkedReactiveVar = new ReactiveVar(false);
  this.someCollection = new Mongo.Collection(null);

  window._template = this;
});

Template.hello.helpers({
  isChecked () {
    return Template.instance().checkedReactiveVar.get();
  },
  inCollection () {
    return Template.instance().someCollection.find({ checked: true }).count() > 0;
  }
});

Template.hello.events({
  'change #checkbox-1'(event, instance) {
    instance.checkedReactiveVar.set(event.currentTarget.checked);
    event.preventDefault();
  },
  'change #checkbox-2'(event, instance) {
    if (event.currentTarget.checked) {
      Template.instance().someCollection.insert({
        checked: true
      });
    } else {
      Template.instance().someCollection.remove({
        checked: true
      });
    }
    event.preventDefault();
  },
});

As you can see, I'm doing two tests, one using a ReactiveVar and the other using a (client-side) Mongo Collection. Neither shows what you are encountering.

In my opinion, using a Mongo Collection only involves a bit more complication from subscriptions. Its fundamental reactivity concept should be identical to using a ReactiveVar. And as you can see with my example, there's nothing wrong with it.

I think it would be much better if you can provide a minimum example to reproduce your issue. On the other hand, I would suggest trying out ReactiveVar in your isChecked template helper, see if the issue is resolved. If so, I suspect something wrong with your mongo collection data.

@Zodiase
Copy link
Owner

Zodiase commented Jan 17, 2017

@matteolavaggi Also what you are experiencing seems to be a completely different issue. It's better for you to create a new issue so we don't sidetrack the conversations here.

@Antoine-O
Copy link

I have done the following concerning initializing switch status , this is the only way I found :/

updateSwitchForValue = function (cssSelector, selectValue, templateInstance) {
    const selectElement = templateInstance.find(cssSelector);
    if (selectElement) {
        if (selectValue) {
            selectElement.MaterialSwitch.on();
        } else {
            selectElement.MaterialSwitch.off();
        }
    } else {
        Meteor.setTimeout(function () {
            updateSwitchForValue(cssSelector, selectValue, templateInstance);
        }, 500);
    }
    return selectElement;
};

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

No branches or pull requests

4 participants