Error on create menu item
lcc91 opened this issue · 9 comments
Bug report
What I did:
1. creating a new menu item without fill any data.
2. I change "Type" to internal link and change back to page link, I found 'page_id' value become 1, then I press 'create' without filled any data again.
What I expected to happen:
1. prompt correct validation message
2. prompt correct validation message
What happened:
1. JS error "An invalid form control with name='' is not focusable."
2. Laravel error : "SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'name' cannot be null"
What I've already tried to fix it:
I think should add some validation rules for "Label" and "Type", but I couldn't found the way to customize it.
Backpack, Laravel, PHP, DB version: backpack 5, laravel 9, php 8.1, mysql 5.7
- Same things happen on https://demo.backpackforlaravel.com/admin/menu-item/create as well
Hello there! Thanks for opening your first issue on this repo!
Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.
Backpack communication mediums:
- Bug Reports, Feature Requests - Github Issues (here);
- Quick help (How do I do X) - Gitter Chatroom;
- Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the
backpack-for-laraveltag;
Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.
Thank you!
--
Justin Case
The Backpack Robot
I am also seeing the same behaviour, with the same message whenever using page_or_link on Backpack V5. The javascript error for each field is:
An invalid form control with name='' is not focusable.
A quick workaround to get the form to submit is to customise the page_or_link.blade.php view file, and remove the required attribute from each of the input elements for page, internal link, and external link.
This obviously requires you to ensure that you validate the request correctly on server-side but it at least allows you to use this package on Backpack V5.
Hello @danielwaghorn thanks for the report.
From my understanding the problem is not in the field itself, it's because when it "errors" (the html validation) cannot properly "validate" the ones that are hidden, is that the problem ? That's why you say that if you remove the required from the fields it works rigth ?
Hi @pxpm, I think that is the case based on my experimentation, so a solution may be to toggle the required attribute on the field when we also toggle d-none based on the Link Type selected.
My only other concern however - and I've not tested this - is as to whether this may still be an issue when the field is not visible, e.g. on a tab which isn't selected.
Hey @danielwaghorn thanks for clarification.
Your concerns are super valid, and I am aware of them.
Now that you talk about it, I've sent a PR Laravel-Backpack/CRUD#2340 sometime ago (sorry it have been a long time) that addresses that tab issue you are mentioning and fixes other error about the save actions dropdown triggering the HTML validation.
Unfortunatelly we didn't prioritize it since there hasn't been much request for it and is a pretty heavy change, people usually is happy validating the fields in a FormRequest, so we moved on. 🤷♂️
This MenuCRUD is a pretty "stationary" package, so I think the appropriate change here is to remove the required from the inputs and add validation in the controller in a FormRequest and be done with it.
If you would like to contribute a PR please feel free to send it and I will happy review it, otherwise I will be moving this to 5.x as it's not priority at the moment.
Cheers
I just submitted a PR that addresses this issue.
#76
Hello everyone, thanks a lot for all the useful information, we will cover this issue to fix it ASAP.
Thanks again.