Skip to content

Conversation

@harryadel
Copy link

template.formId returns the file upload field id not the form id, but form id is a must for validation context to properly grab the form.

@jankapunkt
Copy link
Collaborator

@harryadelb thank you for your contribution. It seems to fix an issue? Would you mind open an issue first which describes the problem and leaves room for discussion? I will take a look at it asap.

@harryadel
Copy link
Author

Hello @jankapunkt I don't think an issue is needed. Basically the validation errors aren't showing up, try grabbing the code in the example and let's say upload a PDF file you would find out that the AutoForm error isn't showing up. It's pretty straight forward.
If an issue is still a must let me know, thank you. :)

@jankapunkt
Copy link
Collaborator

jankapunkt commented Jan 13, 2020

Okay I checked and you are right:

this.formId        = this.data.atts.id;

is not assigning the formId but the input id.

However - I would rather propose to assign the correct formId in onCreated and keep the code in the event handler as before like

ctx = AutoForm.getValidationContext(template.formId);

I also would like to propose to use AutoForm.getFormId() . Something like:

onCreated

this.formId        = AutoForm.getFormId();

What do you think?

@jankapunkt jankapunkt self-requested a review January 13, 2020 08:52
Copy link
Collaborator

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the official AutoForm api (AutoForm.getFormId()) and assign it in onCreated. I already tested it locally and it should work fine.

@harryadel
Copy link
Author

Great suggestion @jankapunkt! Ok, I added a new commit. Please recheck.

Copy link
Collaborator

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@jankapunkt jankapunkt merged commit bc93933 into veliovgroup:master Jan 14, 2020
@jankapunkt
Copy link
Collaborator

@dr-dimitru can you publish a new bumped version to the atmosphere registry, please?

dr-dimitru added a commit that referenced this pull request Mar 2, 2020
- 👨‍💻 Merge #56 Get form Id for validation context thanks @harryadelb
and @jankapunkt
- 📦 Update Atmosphere `ostrio:files` to `v1.13.0`, *was `v1.11.2`*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants