Tradeshift / tradeshift-ui

Tradeshift UI is a framework-agnostic JavaScript library to help Tradeshift App developers to create cohesive user experiences and to provide reusable UI components.
https://ui.tradeshift.com
Other
33 stars 44 forks source link

TS MultiSelect form does not display correct number of selected items #43

Closed cindyvandooren closed 6 years ago

cindyvandooren commented 7 years ago

Priority: medium (currently looking to see if I can find a way around this).

In the document profile generator v4 app, I'm using the multiselect form to set the languages for different document types. The different available languages are added to the form using ng-options. The selected languages are stored in an object, that keeps separate arrays for each document type. So in the select, I use either one of the arrays as the model (depending on the document type).

<form data-ts="Form" name="languageForm">
    <fieldset>
        <label>
            <span>Select Needed Languages</span>
            <select multiple ng-model="selectedLanguages[selectedDocumentType.type]" ng-options="language.label for language in localeCodes" ng-change="setLanguages()"></select>
            <button ts-data="Button" class="ts-primary">
                <span>Set Languages</span>
            </button>
        </label>
    </fieldset>
</form>

I can successfully set the languages (I can see them showing up in the correct array), but when I'm switching between document types I see that the input in the form ("{number of selected items} selected") does not display the correct number of selected languages. An example: The forms model is set to invoice and I set the languages English and Danish. selectedLanguages.invoice = [{English Language Object}, {Danish Language Object}]. The form correctly says that 2 languages have been selected. Then I switch to credit notes. The form's model is now selectedLanguages.creditnote = []. The form now says that 2 languages have been selected, but the model is empty. The form seems to remember the last value that was set.

Including @TimeBomb in case he has something to add (since he helped me look into this issue)

TimeBomb commented 7 years ago

To elaborate, she has two very similar forms - using the same DOM (angular template). There is one for invoices and one for credit notes.

While switching from one document type to another, it looks like it temporarily removes the DOM elements containing the input fields (and the select field she mentioned), then recreates them once the new document type is selected.

If you start at the invoice document type, set a language, then it goes from saying "0 selected" to "1 selected". The problem is that when you switch to credit note, even though the ng-model has changed, it will still say "1 selected" even though there are actually 0 selected. That's the first bug

Continuing on, if she then selects two languages from the "Set languages" select input field, it will correctly say "2 selected". When she switches back to the invoice document type from credit note, the languages input field will still say "2 selected", despite the ng-model having changed and now only 1 is selected. This is a second bug, but seems to be in the same vein as the first bug.

It looks like it's not updating the select field at the right time. I bet if we could forcefully update Spiritual, it would show the correct amount of selected items.

This seems like a bug in Spiritual caused by the way Cindy's updating the DOM and the angular model of the DOM. Her code otherwise works fine, and this doesn't appear to be an issue that's solvable by just forcing an angular digest cycle.

Cindy, if you could post the GitHub branch containing your app's full code, or even better, a heracles sandbox (https://ci-it.ts.sv/view/stack-control/job/create-stack/) with your app running, that'd be helpful.

cindyvandooren commented 7 years ago

I pushed the code to this branch.

The most import files are these:

cindyvandooren commented 7 years ago

Apparently this was due to using ng-hide and ng-show instead of ng-if. Elements were never removed from the DOM. Once I made sure to use ng-if and remove the elements from the DOM this issue did no longer occur.

wiredearp commented 7 years ago

Reopening for investigation of potential bug pending the understanding of what @TimeBomb is saying here (from https://tradeshift.slack.com/archives/dev-ui-components/p1477445116000369).

Actually, while Cindy closed the issue because she figured out a way around it (removing the elements via `ng-if` rather than using `ng-show`), I'm wondering if there's still a bug here.
3:26 To put it very basically, the `ng-model` changed, and though the change was reflected when you CLICKED the select field and the aside was opened, the "X number selected" text visible on the field prior to clicking it did _not_ change, despite the ng-model changing and the angular digest cycle having gone through the motions.
3:27 At face value, it sounds like Spiritual doesn't update the "# of selected item" text until you click that "Okay" (or whatever the text is) button in the aside that pops up upon clicking select inputs.
3:28 When in reality, Spiritual should be able to identify that the selected items in the select field DID change, despite it not being changed via the UI component itself (was changed via angular's ng-model), and it should update appropriately. At the very least, the developer should be able to tell Spiritual to update the "# selected items", though ideally it would just work without having to do anything fancy.
sampi commented 6 years ago

Fixed in #628. Closing.