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

Reset cut is not updating view #689

Open
EdwardMoyse opened this issue Sep 24, 2024 · 4 comments
Open

Reset cut is not updating view #689

EdwardMoyse opened this issue Sep 24, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@EdwardMoyse
Copy link
Collaborator

EdwardMoyse commented Sep 24, 2024

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 EdwardMoyse added the bug Something isn't working label Sep 24, 2024
@EdwardMoyse
Copy link
Collaborator Author

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
Copy link
Collaborator Author

EdwardMoyse commented Oct 1, 2024

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
Copy link
Collaborator Author

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

@EdwardMoyse
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant