BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
856 stars 130 forks source link

Checkboxgroup enhancement and oldValue issue #458

Closed alnico2001 closed 3 months ago

alnico2001 commented 6 months ago

The checkboxgroup feature is very handy.

However, it would be convenient and simplify code complexity if it were possible to target and bind to the <input type="checkbox".../> tags using a css selector like that of the 'on' tag {{on '.className' ~doSomething}}.

I have over 100 checkbox search facets that belong to multiple groups; where the groups are not in order or belong to different templates.

Currently I am using {{for... {{if/then}}...}} to bind to a specific checkboxgroup array using data-link="{checkboxgroup...}.

I also have another scenario where I have 3 checkboxgroup arrays shared across 6 tabs with unique templates using 12 data-link="{checkboxgroup...} instances.

It would simplify things if it were possible to wrap code using a few {^{checkboxgroup '.className' ...}} tags where a css selector binds them dynamically. Something like this...which is just like the {^{on...}} tag...

{^{checkboxgroup '.groupA' ~queryParams.groupA}}
{^{checkboxgroup '.groupB' ~queryParams.groupB}} 

     {{for checkboxFacets}}
         <input type="checkbox" data-link="value{val} class{:~data.propClass}" ... />
         ...
     {{/for}}

{{/checkboxgroup}} 
{{/checkboxgroup}}

Here is a simple example of what I am currently doing https://jsfiddle.net/alnico/ofusrk3x/

In addition, (although I don't have an immediate use for)...I can imagine this 'might' be useful for nested {radiogroup...} items too.




Also, related to checkboxgroup...

I need to collect additional information when an 'item' is added/removed from the checkboxgroup array.

The doc says that when a change is made to the checkboxgroup array..."the data property is observably replaced by a new array"

However, when using $.observe {change: 'set'} I was expecting to get the oldValue for use in comparison; but the oldValue is always the same as the current value.

In the attached example, there is code showing this as well.

alnico2001 commented 6 months ago

In addition to the above code example regarding getting oldValue to collect data... This alternative method works fine, but it involves more code and registering many more event handlers...so not ideal in my opinion.

This method uses {on...} and itemVar to retrieve necessary information. <input type="checkbox" data-link="value{:propID} {on ~collectInfo data=~thisItem}" />

Note: I cannot use the {^{on...}} tag to wrap all the code as I cannot access itemVar in that context.

https://jsfiddle.net/alnico/g4fjLtb0/

BorisMoore commented 6 months ago

Thanks for these valuable suggestions and comments. I'm looking into it.... I'll keep you posted.

BorisMoore commented 5 months ago

Here is a version of jsviews.js which includes support for adding a selector argument to the checkboxgroup tag, and similarly, to the radiogroup tag. jsviewsWithSelectorArg on checkboxgroup.js.txt

It also includes a fix for the oldValue.

Let me know if it works for you...

alnico2001 commented 5 months ago

Thanks Boris for adding this and the quick turn around.

The checkboxgroup tag using a css selector works as expected in stripped down testing.

However there is an issue that breaks them (I discovered when I updating my app code; which is now very clean, thank you ;-)

In the code below, if I use one checkboxgroup tag then everything works fine. But if I add a second checkboxgroup tag, propsB, then neither tag works (adding data to the array)...and there are no errors.

I narrowed the problem down to the ^~paramCounts helper. If I remove the ^ then both checkboxgroup tags work. But note, when using the propsA checkboxgroup tag alone, the ^~paramCounts helper worked fine.

Interestingly enough, I cannot reproduce this issue in the test file like that I initially posted with.

{^{checkboxgroup ~qParams.propsA ".propsA"}}
{^{checkboxgroup ~qParams.propsB ".propsB"}}

    {^{for checkboxes ^~paramCounts=~root[~context].paramCounts}}

         {{if PropClass == "propsA" tmpl="propsA_tmpl" /}}
         {{if PropClass == "propsB" tmpl="propsB_tmpl" /}}

    {{/for}}

{{/checkboxgroup}}
{{/checkboxgroup}}

Also, I now see an oldValue ;-)

Thank you!

BorisMoore commented 5 months ago

Thanks for the testing... Would you be able to provide a small sample with the issue you just found - (^~paramCounts etc. or similar) - that I can test...?

alnico2001 commented 5 months ago

Here is something I forgot to mention...

If I move the {^{for checkboxes ^~paramCounts=~root[~context].paramCounts}} loop outside the checkboxgroup tags, everything works.
Does this give you any ideas? Sorry, it doesn't help with testing though.

The development on this app is all local at the moment, including the database...so might be difficult to get you something to test. And my efforts to try to reproduce the issue in a sample aren't working. I will keep thinking about this...

BorisMoore commented 5 months ago

Yes, I understand. But I don't have any immediate idea of what the source of the problem is. So to fix it I really need a HTML page with it's own local Javascript data that I can debug directly. Hopefully you might find a way? :-).

Meantime I have new update I am working on with a couple of minor fixes. I'll send you a copy in the coming days...

alnico2001 commented 5 months ago

Ok, I found problems...

Test file checkboxgroup-selectorTesting.txt works as written with delegation, but breaks in certain scenarios:

Scenario 1

The last checkboxgroup tag of the group (if it has nothing to bind to) breaks other checkboxgroup tags from delegating inside the {^{if show/hide...}} tag. To test: change the checkboxgroup class selector from "moreThings" to "moreThingsX" and click checkbox "Show more things" and then "Playing cards"...notice "Playing cards" is not added to the "moreThings" list at the bottom.

Also, instead, change "sports_a" to "sports_aX"...the "Playing cards" binding works strangely enough.

Scenario 2

If you move the {^{checkboxgroup ~root.qParams.moreThings ".moreThings"}} tag up one line (above "sports_b"), delegation also breaks like Scenario 1.

So the order of the checkboxgroup tags makes a difference (when they shouldn't) and when there is a checkboxgroup tag that has nothing to bind to makes a difference (those tag(s) should be ignored).

      {^{checkboxgroup ~root.qParams.category ".category"}}
      {^{checkboxgroup ~root.qParams.sports_a ".sports_a"}}
      {^{checkboxgroup ~root.qParams.sports_b ".sports_b"}}
      {^{checkboxgroup ~root.qParams.moreThings ".moreThings"}}

         {^{for sports}}

            {{if propClass == 'category' tmpl="category" /}}
            {{if propClass == 'sports_a' tmpl="sports_a" /}}
            {{if propClass == 'sports_b' tmpl="sports_b" /}}

         {{/for}}

      {{/checkboxgroup}}
      {{/checkboxgroup}}
      {{/checkboxgroup}}
      {{/checkboxgroup}}
alnico2001 commented 5 months ago

Doh, I messed up... Of course if you set "moreThings" to "moreThingsX" it won't bind.

But I took out a piece of code last minute that should have been int there... If you add this tag {^{checkboxgroup ~root.qParams.moreThings2 ".moreThings2"}} below ".moreThings" and a closing tag...it breaks binding on "moreThings". (it has nothing to bind to)

Array moreThings2 is already in the file I posted.

Sorry for the mess up ;-)

BorisMoore commented 5 months ago

No problem. The good news is that I may already have a fix. It looks like it is covered by some additional updates I mentioned I am working on. Sorry I had not yet provided you with that update. I'll send you what I have soon - but it is 'in progress'. I am still validating the similar scenarios for radio buttons.

BorisMoore commented 5 months ago

I have an update available now for testing.

It should work for checkboxgroups or radiogroups with the additional selector arg for filtering. jsviewsWithSelectorArgs-checkboxgroup-radiogroup.js.txt

In addition it should be able to accept live changes to content (and hence to <input>s included in the group,) triggered by {^{if}} or {^{for}}, even if the if/for tag is nested more deeply, and does not have the same immediate parent element as the checkboxgroup/radiogroup.

checkboxgroup-radiogroup-testing.txt

For example:

     {^{checkboxgroup ~root.qParams.category ".category"}}
         <span data-link='{checkboxgroup ~root.qParams.sports_a ".sports_a" disabled=disableSports_a}'>
            {{include}}
               {^{checkboxgroup ~root.qParams.sports_b ".sports_b"}}
                  <div>
                     <span data-link='{checkboxgroup ~root.qParams.moreThings ".moreThings"}'>
                        {{include}}
                           {^{for sports}}
                              {{if propClass == 'category' tmpl="category" /}}
                              {{if propClass == 'sports_a' tmpl="sports_a" /}}
                              {{if propClass == 'sports_b' tmpl="sports_b" /}}
                           {{/for}}
                        {{/include}}
                     </span>
                  </div>
               {{/checkboxgroup}}
            {{/include}}
         </span>
      {{/checkboxgroup}}

Let me know if you see issues...! Thanks...

alnico2001 commented 5 months ago

In my app where I have nested {^{checkboxgroup...".selector"}}...they all work as expected when initializing/disposing of data blocks and the issue with ^~paramCounts not surprisingly has disappeared :-) I also now have {^{radiogroup...".selector"}} implemented, (but not in a nested context)...they work fine too.

However, the example code you posted presents issues when data-link is used...

On page load if you click a sports_a item, binding does not work. However, on page load if you FIRST click a category item and THEN a sports_a item, then sports_a binds surprisingly.

Moreover, this simplified block of code does not work at all...

<span data-link="{checkboxgroup ~root.qParams.category '.category'}">
   {^{for sports}}
      {{if propClass == 'category' tmpl="category" /}}
   {{/for}}
</span>

Also, I flipped all checkboxgroup/radiogroup and type="checkbox/radio" for testing...and the exact same issue with data-link exists for radio's...so that is good that there is the same/consistent issue there too.

Thanks!...getting closer ;-)

UPDATE / Clarification

In the file you supplied, everything works, checkboxgroup-radiogroup-testing.txt

However, I copied the For example {^{checkboxgroup...}} block into my original testing file and that's where I saw the issue mentioned above.

The issue lies here:

This line from your test file works... <label><input type="checkbox" value="{{:propID}}" class="{{:propClass}}" /> {{:name}}</label> {{:propClass}}

while this line from my test file does not work. <label><input type="checkbox" data-link="value{:propID} class{:propClass}"/> {{:name}}</label> {{:propClass}}

My test file: checkboxgroup-selectorTesting.txt

BorisMoore commented 5 months ago

I'm having difficulty replicating (or maybe understanding) your issues:

You say "when data-link is used..." sports-a does not bind. But if I set disableSports_a: false, and then load the page, using this data-link markup:

<span data-link='{checkboxgroup ~root.qParams.category ".category"}'>
   <span data-link='{checkboxgroup ~root.qParams.sports_a ".sports_a" disabled=disableSports_a}'>
      <span data-link='{checkboxgroup ~root.qParams.sports_b ".sports_b"}'>
         <span data-link='{checkboxgroup ~root.qParams.moreThings ".moreThings"}'>
            {^{for sports}}
               {{if propClass == 'category' tmpl="category" /}}
               {{if propClass == 'sports_a' tmpl="sports_a" /}}
               {{if propClass == 'sports_b' tmpl="sports_b" /}}
            {{/for}}
         </span>
      </span>
   </span>
</span>

then everything seems to me to work correctly, including initial sports_a binding.

One thing that can lead to issues on initial load is if you are using data-linking on the inputs to set value and class, within a data-linked checkboxgroup:

<span data-link="{checkboxgroup ~root.qParams.category '.category'}">
   {^{for sports}}
      <input type="checkbox" data-link="value{:propID} class{:propClass} "/>...
   {{/for}}
</span>

In fact the data-linking (initial binding) of the outer element (<span>) will occur before the data-linking/binding of the <input>. So here, the checkbox will not find any inputs with class "category" during initial binding.

Instead, better to use:

<span data-link="{checkboxgroup ~root.qParams.category '.category'}">
   {^{for sports}}
      <input type="checkbox" value="{{:propID}}" class="{{:propClass}}"/>...
   {{/for}}
</span>

That way the value and class are set during rendering, and before binding....

Your simplified version:

<span data-link="{checkboxgroup ~root.qParams.category '.category'}">
   {^{for sports}}
      {{if propClass == 'category' tmpl="category" /}}
   {{/for}}
</span>

seems to work for me, but here you are of course only binding the category, so checkboxes with class moreThings will not bind to anything...

If I am missing something, maybe you can provide a complete sample showing the issue(s), plus repro steps, to make sure I am testing the same thing as you.

Thanks...!

Ah - I just sent this without having seen your UPDATE

BorisMoore commented 5 months ago

OK - I just read your update. So you are seeing the same issue I suspected of (as explained in my last comment) for nested data-linking and the processing model/sequence....

So once you have reverted to the suggested non-data-linked verion, does everything now work for you?

alnico2001 commented 5 months ago

Yes, everything works correctly when not using data-link.

Good note: "nested data-linking and the processing model/sequence..."

Sorry for the mid-update message and sending you down a rabbit hole ;-)

Thank you for adding this Boris, this is awesome!

Brent

BorisMoore commented 5 months ago

So once you have reverted to the suggested non-data-linked verion, does everything now work for you?

What I meant was to revert to the non-data-linked approach for value and class on <input> tags.

Not suggesting (of course) that you revert to non-data-linked {checkboxagroup} etc. It's good to test both data-linked and tag versions of checkboxgroup / radiogroup.

alnico2001 commented 5 months ago

Yeah...I meant not using data-link for (value and class) in my test file regarding checkboxgroup; and thus removed data-link altogether as it wasn't being used for anything else.

In my app I am using and tested <input class="{{:class}}" value="{{:value}}" type="checkbox" data-link="disabled{:...}" />.

All good ;-)

alnico2001 commented 5 months ago

Another update... I converted 6 checkboxgroup TAGS in my app to use data-link="{checkboxgroup...}"...they all worked as expected.

Sorry, I am juggling to many things and testing too ;-)

BorisMoore commented 4 months ago

Hi Brent, I have an update to share with you. It is a candidate for jsviews.js v1.0.14. It includes the features we have been working on, for both checkboxgroup and radiogroup. It also includes a few minor bug fixes, as well as an underlying feature which I used for the checkbox/radio/group tags, so as to allow them to respond to 'dom change' events even when wrapping to a deeper level

jsviews.js v1.0.14 Candidate 4.js.txt

Here is a sample file:

-checkboxgroup-sampleTEST.html.txt

which includes the following nested tags and elements:

  {{myDataChangeTag}}
    {^{checkboxgroup ~excluded ".exclude"}}
      <table class="grid" style="width:600px;">
        <thead>
          <tr>
            <th style="width:100px;" data-link="{on ~sortCol ~sortBy ~reverseSort 'category'}
                class{:~sortBy==='category'?~reverseSort?'filter sort down':'filter sort up':'filter sort'}">
              <input data-link="{:~cat:} class{:~cat?'active':''}" style="width:70px;" />Category
            </th>
            ...
          </tr>
        </thead>
        <tbody>
          {^{checkboxgroup ~taxed ".tax"}}
            {^{for lineItems sort=~sortBy reverse=~reverseSort filter=~category}}
              <tr>
                <td class="editable"><input data-link="category" style="width:80px;" /></td>
                ...
              </tr>
            {{else}}
              <tr><td colspan="6">No items</td></tr>
            {{/for}}
          {{/checkboxgroup}}
        </tbody>
      </table>
    {{/checkboxgroup}}
  {{/myDataChangeTag}}

The underlying feature allows you also to create custom tags which implement onDomChange methods. Calls to those methods bubble up from nested tags such as {^{for}} - illustrated by myDataChangeTag in the sample.

alnico2001 commented 4 months ago

The sample looks great Boris...a lot of features working together, cool ;-) Very nice that a custom tag can pick up dom changes on deeper levels via onDomChange. I found no issues testing candidate 4. Excellent work!

BorisMoore commented 4 months ago

Thanks Brent. Here is a new version of the sample page, which now includes radiogroups, using the new features.

-checkbox-radio-group-sampleTEST.html.txt

alnico2001 commented 4 months ago

Looks good Boris...

I've never seen anything on the site that mentions transitions...so I added something for that. Blinking changes with no movement is great but can easily be missed. I've added a fadeIn() transition for 'Total cost' and 'Compare A/B' items using onBeforeUpdate and onUpdate...either work here... Anyway, something you might want to add.

Updated file: -checkbox-radio-group-transition-sampleTEST.html.txt

BorisMoore commented 4 months ago

Thanks Brent. Yes, it is a good idea to be able to add transitions. I'll send you a modified version soon....

BorisMoore commented 4 months ago

Here is a modified version of your sample file:

-checkbox-radio-group-transition-sampleTEST2.html.txt

For the CompareA/B displays I have used onBeforeChange (rather than onUpdate), following your approach. For the others I have used converters, rather than onBeforeChange, to first test whether there is actually a change, and if so, trigger a transition.

alnico2001 commented 4 months ago

Nice...transitioning only the changed values using converters. Nice to see various methods here. I cannot think of anything more to add, I promise ;-)

BorisMoore commented 3 months ago

This is being published with the new v1.0.14 release.

Thanks for you help on this, Brent...