microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
964 stars 309 forks source link

[BUG] Unable to change user-id #2831

Closed grosch-intl closed 11 months ago

grosch-intl commented 1 year ago

Describe the bug Using mgt-person inside of an Angular wrapper, changing the value assigned to the user-id attribute does not trigger a change in the person component.

To Reproduce

import {Component, CUSTOM_ELEMENTS_SCHEMA} from '@angular/core'

@Component({
    selector: 'app-example',
    standalone: true,
    template: '<mgt-person [attr.user-id]="email"></mgt-person><button click="click()">Change</button>',
    schemas: [CUSTOM_ELEMENTS_SCHEMA]
})
export class ExampleComponent {
    email = 'somebody@somewhere.com'

    protected click() {
        this.email = 'somebody.else@somewhere.com'
    }
}

Expected behavior When the value bound to the attribute changes, the mgt-person item should update to the new value.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

Additional context Add any other context about the problem here.

sebastienlevert commented 1 year ago

This is a good question. I tried and it's not related to Angular per se, but to the implementation of the component. I want to learn more about the scenario to swap the user email / id after it was rendered vs. rendering a new instance of the component? Right now we do not request a state update on any of the properties as we think of this component to be "read-only". With your scenario in mind, it will help us understand and identify a solution or suggest changes to your implementation.

Thanks!

grosch-intl commented 1 year ago

I have a field for who manages the space, and I use mgt-person to show who that is. Certain people have the ability to change that, and so I add a pencil icon next to it as an "edit". They pick a new person, and at that point I need the page to show that updated owner.

Another part of the page lists document owners, and people filling out the checklist will add those people. It's normal to go in and change who owns it, and they may be updating multiple people.

Those changes aren't actually applied to the server until they hit the Save button, which they might not want to do right now, and so they have no way of seeing the edits they've done so far to owners/contacts.

sebastienlevert commented 1 year ago

Couldn't you re-render the parts of the UI where data changed to reflect the changes visually? Actually... That makes me think that this could be more of a React-way of thinking compared to WC or Angular specifically... @gavinbarron thoughts on that?

grosch commented 1 year ago

Sure, but having to manually remove a component from the DOM just to add the same component back in with a different attribute is using a pretty big hammer to get the display to update.

gavinbarron commented 1 year ago

My opinion is that changing the user-id attribute should absolutely trigger a re-render. It's an input to which we should be responding to changes on.

Thanks for raising this @grosch-intl

sebastienlevert commented 1 year ago

In this case, let's make user-id, person-query and person-details all trigger a re-render.

grosch-intl commented 1 year ago

@Mnickii Any ETA on when this will be resolved?

Mnickii commented 1 year ago

Hi @grosch-intl, a fix for this issue will be included in the next major release of MGT, possibly by the end of the year.

sebastienlevert commented 1 year ago

It will be available soon as part of the next version published to npm, though. npm i @microsoft/mgt-components@next for instance.