Meteor-Community-Packages / meteor-autoform

AutoForm is a Meteor package that adds UI components and helpers to easily create basic forms with automatic insert and update events, and automatic reactive validation.
MIT License
1.44k stars 328 forks source link

afQuickField id field is overwritten - v7 #1696

Closed lynchem closed 3 years ago

lynchem commented 3 years ago

This issue was introduced in 7, working previously.

We have a modal with a datetimepicker in it (eonasdan-bootstrap-datetimepicker) which is hidden. Before showing the modal we try and set the date in the picker but the picker has not been initialised so fails.

The template that the modal contains has been rendered so I can only imagine that there might be an optimisation to not init controls that are not visible somewhere in 7?

jankapunkt commented 3 years ago

thank you @lynchem is this the correct repo: https://github.com/Eonasdan/bootstrap-datetimepicker ?

Do you activate the modal during the form using a custom mechanism or this part of the mentioned repo? Once I get a reproduction I can fix it asap.

lynchem commented 3 years ago

Nothing to do with the datetimepicker repo AFAIK and yes, that's the correct repo. Actually, thinking about it a bit more, this is most likely an issue in how the plugins interact with AF. So in this case there's an atmosphere package https://atmospherejs.com/aldeed/autoform-bs-datepicker which should be creating the datetimepicker. We have a local modified copy of this package (to fix some TZ issues) so I'll look and see if I can figure anything more

jankapunkt commented 3 years ago

Acutally we have this package also now under the MCP hood: https://github.com/Meteor-Community-Packages/meteor-autoform-bs-datepicker :-) If there is anything causing trouble I could fix that, too. Publishing would be then just a few steps away

lynchem commented 3 years ago

Right Jan, I'll make a mental note - I should always start with the simplest thing that could be going wrong. :smile:

When you create a control like this:

{{> afQuickField id="disbursement-paid-date" name='date'}}

It seems in v7 is replaces the id leaving you with image

where it used to leave that alone and you'd have image

I'm guessing I'm not alone in having using the id= field occasionally to reference a control so might be a good idea to leave that as it was. Or add it to the breaking changes. It's simple enough to fix.

I'll change the name of the issue to reflect this.

jankapunkt commented 3 years ago

Thanks for this investigation @lynchem Removing the id is of course not an intended behaviour at all. I will get to the bottom of this and inform you, once it's fixed and added to the PR.

jankapunkt commented 3 years ago

Hi @lynchem I found and fixed this issue. It has been removed during the rewrite. I reversed this but at the same time also support the id setting mechanism that @pouya-eghbali has introduced. Can you verify it's working now using a local copy of the v7-dev branch? Alternatively I could also publish another rc version.

lynchem commented 3 years ago

Great. Would be handy if you can publish it. I'll try have a look over the weekend

jankapunkt commented 3 years ago

Published as 6.4.0-rc.7.0.1, you need to set the version explicitly via aldeed:autoform@6.4.0-rc.7.0.1 or it won't be updated.

lynchem commented 3 years ago

Ok, I can confirm it's fixed in 7.0.1 :tada: