phoenix icon indicating copy to clipboard operation
phoenix copied to clipboard

Reset cut is not updating view

Open EdwardMoyse opened this issue 1 year ago • 4 comments

See: https://github.com/user-attachments/assets/beb7359b-fa5a-4326-beff-38dddd8d3d5e

But basically, if you modify the cuts, then click 'Reset', the cut.model is updated but not the view (you get in the situation where you are turning a checkbox on, but it is already enabled by being reset)

This is WAY outside my comfort zone, but I wonder if the issue is in code like `` that we have:

              <div *ngSwitchCase="'rangeSlider'" class="range-slider">
                <div class="range-slider-inputs d-flex justify-content-between">
                  <mat-checkbox
                    [checked]="config.enableMin" 
                    (change)="config.setEnableMin($event.checked)"
                  ></mat-checkbox>

whereas we should be using a two way binding, i.e. using ngModel

              <div *ngSwitchCase="'rangeSlider'" class="range-slider">
                <div class="range-slider-inputs d-flex justify-content-between">
                  <mat-checkbox
                    [ngModel]="config.enableMin" 
                    (change)="config.setEnableMin($event.checked)"
                  ></mat-checkbox>

Unfortunately this doesn't seem to work, so I guess it's not that easy....

Pinging @9inpachi and @emiliocortina, who also know WAY more about Angular than me....

EdwardMoyse avatar Sep 24 '24 13:09 EdwardMoyse

I tried to debug this in: https://github.com/HSF/phoenix/tree/main-debug-refreshing-views

My idea was that we could use the ChangeDetectorRef in PhoenixMenuItemComponent to force the view to be redrawn.

To do this, I added a new method:

  viewUpdate(): void {
    this.cdr.markForCheck();
    this.calculateConfigTop();
  }

I also changed the html to also call the new method viewUpdate:

                (click)="config.onClick(); viewUpdate()"

But this does not work as I would expect:

  • when I click on gear next to the "Cut Options" e.g. for Tracks, then calculateConfigTop is called.
  • if I e.g. disable the min phi cut, the cut is updated as expected.
  • if I click "Reset Cuts" then the onClick function defined in PhoenixMenuUI is called, followed by the new viewUpdate method.
  • However the view is not redrawn (the checkbox is still unclicked, and any modified cuts are not shown to have reset (but I confirmed that the cut itself has been reset).

My best guess is that this is because I'm invalidating the parent and not the child view. But I'm not sure what else to try...

EdwardMoyse avatar Oct 01 '24 13:10 EdwardMoyse

Right. If I break in viewUpdate then I see that this PhoenixMenuItemComponent has no children, but it DOES have the configs which correspond to the cuts.

Screenshot 2024-10-01 at 17 08 21

And the are turned into the GUI in phoenix-menu-item-component.html:

        <ng-container
          *ngFor="let config of castConfigsToAny(currentNode.configs)"
          [ngSwitch]="config.type"
        >

So I cannot work out why this is not being redrawn. :-(

EdwardMoyse avatar Oct 01 '24 15:10 EdwardMoyse

Hmm. I just noticed that _attachedToViewContainer is false for the ChangeDetectorRef... but not sure if this is relevant or not...

EdwardMoyse avatar Oct 01 '24 15:10 EdwardMoyse

@9inpachi @emiliocortina if either of you have any time to look at this I would be very grateful, because I'm pretty lost.

EdwardMoyse avatar Oct 01 '24 15:10 EdwardMoyse

I have published a pull request that I think fixes the issue. Please check and let me know, I'm a bit unfamiliar with the code I don't want to cause side effects... some of the commands were actually failing on my local setup (I was only able to run yarn start). One of the issues was that the function addConfig was creating a copy of the objects, instead of storing references, so I did a little refactor of that.

emiliocortina avatar Oct 20 '24 22:10 emiliocortina

Fixed! Thanks @emiliocortina

EdwardMoyse avatar Nov 08 '24 21:11 EdwardMoyse