Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afQuickField id field is overwritten - v7 #1696

Closed
lynchem opened this issue Nov 3, 2020 · 9 comments · Fixed by #1694
Closed

afQuickField id field is overwritten - v7 #1696

lynchem opened this issue Nov 3, 2020 · 9 comments · Fixed by #1694
Assignees

Comments

@lynchem
Copy link

lynchem commented Nov 3, 2020

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?

@lynchem lynchem changed the title DatePicker in hidden modal not initialised - v7 DateTimePicker in hidden modal not initialised - v7 Nov 3, 2020
@jankapunkt jankapunkt linked a pull request Nov 3, 2020 that will close this issue
10 tasks
@jankapunkt
Copy link
Member

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.

@jankapunkt jankapunkt mentioned this issue Nov 3, 2020
10 tasks
@lynchem
Copy link
Author

lynchem commented Nov 3, 2020

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
Copy link
Member

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
Copy link
Author

lynchem commented Nov 3, 2020

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

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.

@lynchem lynchem changed the title DateTimePicker in hidden modal not initialised - v7 afQuickField id field is overwritten - v7 Nov 3, 2020
@jankapunkt
Copy link
Member

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 jankapunkt self-assigned this Nov 3, 2020
@jankapunkt
Copy link
Member

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
Copy link
Author

lynchem commented Nov 13, 2020

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

@jankapunkt
Copy link
Member

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
Copy link
Author

lynchem commented Nov 16, 2020

Ok, I can confirm it's fixed in 7.0.1 🎉

@lynchem lynchem closed this as completed Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants