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

Add focus and blur events + viewer event documentation #841

Merged
merged 4 commits into from
Oct 16, 2023

Conversation

Skaiir
Copy link
Contributor

@Skaiir Skaiir commented Oct 13, 2023

Closes #60
Related to #839

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Oct 13, 2023

Both events receive `{ data, errors }` as a payload.
### `submit :: { data, errors }`
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, short format to visualize this! Do we have any other place where we use this syntax or how did you come up with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be fair there isn't a standard in place yet. Although, I didn't want to fill the documentation with a lot of events and descriptions, especially if we do the right thing and do more event driven stuff in the future. That's also why I only add descriptions when the event name doesn't make it entirely obvious.

We can always ask for GPT's opinion on this, while keeping the constraints of the compact format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GPT didn't seem to mind :D

packages/form-js-viewer/test/spec/Form.spec.js Outdated Show resolved Hide resolved
@Skaiir Skaiir force-pushed the 60-field-focus-blur-events branch from 0dbdf39 to a09e9f6 Compare October 16, 2023 06:31
Comment on lines +180 to +181
- `formField.add`
- `formField.remove`
Copy link
Contributor

Choose a reason for hiding this comment

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

These are editor events, correct? Maybe we can add the full event documentation to the general form-js README?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually no these events are bound to the form field registry. So they technically are viewer events, they would fire during import.

Copy link
Contributor Author

@Skaiir Skaiir Oct 16, 2023

Choose a reason for hiding this comment

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

I think it's still good to separate them, as you access them differently: form.on() and formEditor.on(). We separate the API too so, IMO makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can programmatically add fields to the viewer in theory, there may be other use cases we haven't thought of that do this. For example, a streamed version of the form generator using only a viewer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

@Skaiir Skaiir merged commit 6506118 into develop Oct 16, 2023
10 checks passed
@Skaiir Skaiir deleted the 60-field-focus-blur-events branch October 16, 2023 12:35
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 16, 2023
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.

Make it possible to listen to on focus events
2 participants