openwisp/django-loci

[ux] errors made while adding a new device gets hidden

atb00ker opened this issue ยท 9 comments

When adding a new device if someone makes an error of not entering Geometry for a Outdoor environment, the user will get error Please correct the errors below. at the top of the page but the actual field to make the correction is not shown.
The user needs to change the Type field then select Outdoor environment again to see the errored field.
This may cause confusion to users.

I don't know jquery so I am stopping working on it, but I found that you have to do something similar for outdoor like it was done here:
https://github.com/openwisp/django-loci/blob/master/django_loci/static/django-loci/js/loci.js#L328-L332

@atb00ker can I take on this, from what I guess i will need to include the appropriate js code in the change_form template and then make the section visible. Let me know if I am on track

@muarachmann I can't be sure of the approach without looking at the pull request, but if you want to take it, go for it!

Hello, I did some research and found that the code that @oltarzewskik said isn't actually the best answer, since the required field isn't just .field-indoor (User can actually set the indoor coordinates and floor, but leaving the geographic information empty. When user save, the user still will get error Please correct the errors below. at the top of the page but the actual field to make the correction is not shown, and somehow the Floorplan selection is showed, where it shouldn't be appeared).
What do you think? @atb00ker

@marfgold1 I can't be 100% sure of the solution without investigating the bug but what solution do you think would work?

I think most probably in the clean() of this form, we should add a condition: if outdoor: raise exception

@atb00ker also I've found the bug when type is selected and isMobile is toggled on, but I solved it too. Should I create an issue for this, or create another pull request, or just add another commit in the same pull request? I think it's should be in the same commit, as that was also part of the .loci.coords fieldset.

@marfgold1 if you found another bug, please open another bug report (github issue).

@nemesisdesign i do actually open some issues ๐Ÿ˜ƒ. Please also check #34 #35 #36 , I already told @atb00ker and he said he want to wait for your review. Thank you ๐Ÿ˜„