feat(editors/ied): Improve IED editor UI for IED and LN selection

[danyill]

Closes #1287

This PR does three things to improve usability of the editing and selection interface in the IED editor:

• If you select an IED and open the IED selection and select the same IED, nothing changes (previously it would reset the selections)
• If you edit a DAI value, the selected IED is not reset
• Sorts the logical node classes in the class filter
• Defaults to showing no logical node classes selected on first opening of logical node classes filter

[trusz]

I just only have a few nitpicks (tagged with Nit:) and a question (tagged with Q:)
They are not critical so you can ignore some or all if you'd like 😄

[trusz]

I am testing the fix and would like to get your feedback if I understood the bugs correctly.

• If you select an IED and open the IED selection and select the same IED, nothing changes (previously it would reset the selections)

  I was not able to reproduce this one

• If you edit a DAI value, the selected IED is not reset

  Before:
  

  After:
  

• Sorts the logical node classes in the class filter
  &
  Defaults to showing no logical node classes selected on first opening of logical node classes filter

  Before	After

[trusz]

• Defaults to showing no logical node classes selected on first opening of logical node classes filter

  When I open the LN class filter, nothing is selected. If I close without making any selection, the filter will hide everything. Was this intended?

  

[danyill]

When I open the LN class filter, nothing is selected. If I close without

making any selection, the filter will hide everything. Was this intended? Hmmm, no we must not do that - let me take a look. Thanks for testing.

…

On Tue, Aug 22, 2023 at 8:05 PM Tamás Russ ***@***.***> wrote: - *Defaults to showing no logical node classes selected on first opening of logical node classes filter* When I open the LN class filter, nothing is selected. If I close without making any selection, the filter will hide everything. Was this intended? [image: 20230822100224_ied_maybe_bug] <https://user-images.githubusercontent.com/1510715/262279175-bcbcb606-30d2-4daa-b478-59bf5a9e1ee2.gif> — Reply to this email directly, view it on GitHub <#1288 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFEXX3F3UVO6AKSMR223B3XWRR5RANCNFSM6AAAAAA3DVBDXI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

--

-- Private or confidential message? Public Key available here <https://pgp.mit.edu/pks/lookup?search=dan.mulholland%40gmail.com&op=index> :

[trusz]

We usually handle the "nothing-is-selected" case as if everything would be selected. Would that work for you here?

[danyill]

Oh yes that is how it must be

…

On Tue, 22 Aug 2023, 21:22 Tamás Russ, ***@***.***> wrote: We usually handle the "nothing-is-selected" case as if everything would be selected. Would that work for you here? — Reply to this email directly, view it on GitHub <#1288 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFEXX5B2IHKHZ5MK2QBK7LXWR263ANCNFSM6AAAAAA3DVBDXI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[danyill]

If you select an IED and open the IED selection and select the same IED, nothing changes (previously it would reset the selections)

I was not able to reproduce this one

Since video-chat is our new paradigm, here we are 😉

reset-selection.mp4

[trusz]

If you select an IED and open the IED selection and select the same IED, nothing changes (previously it would reset the selections)

I was not able to reproduce this one

Since video-chat is our new paradigm, here we are 😉
reset-selection.mp4

I see, thank you for the clarification!
The description did not mention that it resets the LN filter. I thought it would reset the IED selection.

Now I could replicate it and can confirm that the PR fixed it 😄

[danyill]

We usually handle the "nothing-is-selected" case as if everything would be selected. Would that work for you here?

That is what we must have. I have updated and added a test for it.

[trusz]

I find a behavior a bit surprising. It selects everything if I don't have anything selected and not just behaves as-if.

See screen recording

However, I think the PR is at an acceptable place and If you do not want to change this anymore you are welcome to merge it. 👍

[danyill]

There is a few odd things with this code:

I'm surprised there is a this.requestUpdate('selectedIed'); in the LN class filtering. Perhaps it should be:

this.requestUpdate('selectedLNClasses');

But it should be unnecessary anyway (it's a property).

I had thought I would be able to do something like:

<oscd-filter-button
            id="lnClassesFilter"
            multi="true"
            .header="${translate('iededitor.lnFilter')}"
            @selected-items-changed="${(e: SelectedItemsChangedEvent) => {
              let selectedItems = e.detail.selectedItems;
              const nothingSelected = selectedItems.length === 0;
              if (nothingSelected) {
                selectedItems = this.lnClassList.map(
                  lnClassInfo => lnClassInfo[0]
                );
              }

              this.selectedLNClasses = selectedItems;
              this.requestUpdate('selectedIed');
              this.lNClassListOpenedOnce = true;
            }}"
          >
            <span slot="icon">${getIcon('lNIcon')}</span>
            ${this.lnClassList.map(lnClassInfo => {
              const value = lnClassInfo[0];
              const label = lnClassInfo[1];
              return html`<mwc-check-list-item
                value="${value}"
                ?selected=${this.lNClassListOpenedOnce &&
                this.selectedLNClasses.includes(value) &&
                this.selectedLNClasses.length !== this.lnClassList.length}
              >
                ${label}
              </mwc-check-list-item>`;
            })}
          </oscd-filter-button>

Where this.selectedLNClasses.length !== this.lnClassList.length provides the behaviour you are looking for however for reasons unclear to me there is no reactive update when I add this.

The easiest way I can get this behaviour is by doing the following within oscd-filter-button.ts and it seems if that is the behaviour we are looking for then it would apply across all usages of these elements.

  private onClosing(): void {
    const selectedItems: string[] = [];
    if (this.selected) {
      if (this.selected instanceof Array) {
        this.selected.forEach(item => selectedItems.push(item.value));
      } else {
        selectedItems.push(this.selected.value);
      }
      if (this.items.length === selectedItems.length) {
        this.items.forEach(item => (item.selected = false));
        return;
      }
      this.dispatchEvent(newSelectedItemsChangedEvent(selectedItems));
    }
  }

However there may be use cases for selecting nothing and being able to see that (in the IED editor this shows the IED structures without any of the LNs for instance which may be a feature worth having -- ??) so it's not obvious that this is generic (although with this change all tests would currently pass for open-scd but that may say more about the adequacy of our tests for these kind of changes than anything else).

I suggest we merge as is as the behaviour is a significant improvement on what it was before and it's not clear to me how to implement that functionality you've requested for just this usage of the oscd-filter-button. However happy to be mentored in this regard 😉

[trusz]

There is a PR for PR to be checked out first:

• ref: separate ui and business logic danyill/open-scd#3

[trusz]

OpenSCD is now in packages/open-scd
Need help with the changes?

[danyill]

Thank you for your patience and for showing me a better way to do this.

OpenSCD is now in packages/open-scd Need help with the changes?

Surprisingly, achieved with no conflicts or test failures!