search

SamProf / MatBlazor

Material Design components for Blazor and Razor Components
http://www.matblazor.com
MIT License
2.84k stars 385 forks source link

Cannot read property 'classList' #651

Open gratiaterra opened 4 years ago

gratiaterra commented 4 years ago

Describe the bug Click a MatButton that removes an item from a list bound in MatSelect, then sets a selected index on MatSelect to a valid index in the list. Page load crashes

To Reproduce

Yes, remove item

async Task Delete() { matSelectedList.Remove(selectedItem); selectedItem=matSelectedList.FirstOrDefault(); StateHasChanged(); //crash }

This was not seen until a recent update to latest nuget.

** Crash: Microsoft.JSInterop.JSException: Cannot read property 'classList' of undefined blazor.webassembly.js:1 TypeError: Cannot read property 'classList' of undefined blazor.webassembly.js:1 at Object.removeClassAtIndex (http://localhost:53414/_content/MatBlazor/dist/matBlazor.js:1:207672) blazor.webassembly.js:1 at e.setSelectedIndex (http://localhost:53414/_content/MatBlazor/dist/matBlazor.js:1:62195) blazor.webassembly.js:1 at e.setValue (http://localhost:53414/_content/MatBlazor/dist/matBlazor.js:1:62679) blazor.webassembly.js:1 at e.set [as value] (http://localhost:53414/_content/MatBlazor/dist/matBlazor.js:1:204199) blazor.webassembly.js:1 at Object.e.setValue (http://localhost:53414/_content/MatBlazor/dist/matBlazor.js:1:199107) blazor.webassembly.js:1 at http://localhost:53414/_framework/blazor.webassembly.js:1:9805 blazor.webassembly.js:1 at new Promise () blazor.webassembly.js:1 at Object.beginInvokeJSFromDotNet (http://localhost:53414/_framework/blazor.webassembly.js:1:9773) blazor.webassembly.js:1 at _mono_wasm_invoke_js_marshalled (http://localhost:53414/_framework/wasm/dotnet.3.2.0.js:1:171294) blazor.webassembly.js:1 at do_icall (:wasm-function[6049]:0x10f8b1) blazor.webassembly.js:1 at System.Threading.Tasks.ValueTask`1[TResult].get_Result () <0x42e1888 + 0x0002c> in :0 blazor.webassembly.js:1 at MatBlazor.BaseMatComponent.JsInvokeAsync[T] (System.String code, System.Object[] args) <0x47ea1b8 + 0x000d6> in :0

fededim commented 4 years ago

I am experiencing the same issue, the error happens when you change both the binded value and the item list. As a workaround you have to call twice StateHasChanged, first change the binded value, call StateHasChanged, then update the item list and recall StateHasChanged.

Christian-Oleson commented 4 years ago

I've not had time yet to do anything with this except confirm it is an issue (I found the issue when another one of my dev co-workers was asking why the select wasn't working).

@fededim or @gratiaterra , are either of y'all interested in doing the work to resolve this one?

Christian-Oleson commented 4 years ago

@enkodellc , is it possible to get a label on this one (and other tickets) where we have confirmed the issue exists?

enkodellc commented 4 years ago

@Christian-Oleson "bug" is there so in my opinion if you come across something that is not a bug then I would change it to something different. Just my 2 cents, use what is there instead creating more labels.

naretto commented 4 years ago

Found this bug today while using MatSelectValue too.

Tried calling StateHasChanged(); twice as suggested above, but still got error anyway, so workaround hasn't worked for me.

Edit: Apparently, error only happens when previously selected index was not zero. If it was zero, it works fine

dorana commented 4 years ago

Opened a duplicate of this issue. But transferring my workaround here and closing my Issue.

Workaround done by compiling js from Github source and using this JS file instead of the one bundles in the NuGet, then adding null check

removeClassAtIndex: function (e, n) {
    if (t.menu.items[e] != null && t.menu.items[e].classList != null) {
        t.menu.items[e].classList.remove(n);
    }
}

as well as

setAttributeAtIndex: function (e, n, i) {
    if (t.menu.items[e] != null) {
         t.menu.items[e].setAttribute(n, i);
    }
}
Christian-Oleson commented 4 years ago

Opened a duplicate of this issue. But transferring my workaround here and closing my Issue.

Workaround done by compiling js from Github source and using this JS file instead of the one bundles in the NuGet, then adding null check

removeClassAtIndex: function (e, n) {
    if (t.menu.items[e] != null && t.menu.items[e].classList != null) {
        t.menu.items[e].classList.remove(n);
    }
}

as well as

setAttributeAtIndex: function (e, n, i) {
    if (t.menu.items[e] != null) {
         t.menu.items[e].setAttribute(n, i);
    }
}

Would you be able to open a PR to fix this in the next release? That would be helpful!

naretto commented 3 years ago

I finally had some time to look into this today and it is not as easy to fix.

I looked into the MatSelect code, looking for those JavaScript functions and could not find them (other than in the minified JS file).

MatSelect is only wrapping the MDC Select component from Material itself and those functions come from there:

The wrapping targets material version 7.0.0, which has those functions. However, material has now moved to version 8.0.0 and those functions were entirely removed.

That is listed under the breaking changes: https://github.com/material-components/material-components-web/blob/v8.0.0/CHANGELOG.md

So long story short the bug originates from a dependency and that dependency has moved on. We could fix the minified JS in MatBlazor, but that's not a proper solution as any time the file is regenerated, the fix would be lost.

I think for now, I will do the same as suggested by dorana and just fix my JS file and point to it instead of package one.

Maybe there is a better solution, like override the methods coming from the dependency. Not sure (maybe MatBlazor will move to Material 8 so this becomes a nonissue?).

Edit: Yes, it seems the packages from material will be updated to version 8 in the future: https://github.com/SamProf/MatBlazor/commit/9a35f3eadebd34c71395ee48b8421b600a8b6d44

So this will need retesting after this version is out and this bug may or may not still be around.

SamProf commented 3 years ago

I will update MatBlazor to latest MDC 10.0 in very nearest time.

RoelHermans commented 3 years ago

If your MatSelect's are generated by some loop, I managed to fix it by applying the @key="selectedItem" parameter to the looped component which has the MatSelect.