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

Job submission process should involve initial input validation beyond HTML form validation #218

Closed
marisademeglio opened this issue Apr 15, 2024 · 11 comments
Milestone

Comments

@marisademeglio
Copy link
Member

This would make job submission a 2-step process.

Related to #204

@bertfrees
Copy link
Member

@marisademeglio Could you elaborate? It's unclear for me what this would mean in practice.

@marisademeglio
Copy link
Member Author

See #204 (closed just because this issue replaces it).

Right now:

Job form button "Run" => HTML form validation fails => Feedback to the user

Menu item "Run" => Submits the job => It fails and gives a confusing error message

What we should do instead:

Run job invoked via button or menu => Validate the job values ourselves, see that it fails => Display feedback

@bertfrees
Copy link
Member

In #204 you said:

Need to invoke form validation when Ctrl+R is pressed

but later you said:

We should probably switch to custom validation that is invoked a deeper level than HTML form validation, and use it everywhere.

I don't understand why form validation is not sufficient. If the user hits Ctrl+R, he is always on the job submission page, right? So why can't Ctrl+R simply be a shortcut for the "Run" button?

@bertfrees
Copy link
Member

I don't know how else to explain it except to say that it doesn't work that way in Electron?

Ok, that's fair enough.

The reason I initially asked for clarification was because I thought this perhaps involved some additional validation that we didn't have today, such as validation of the input files (which would require an engine change, see also daisy/pipeline-framework#136). But so it's nothing like that...

there's no point in rehashing it.

I'm sorry 😬. To me this issue seemed pretty much a continuation of #204.

@marisademeglio
Copy link
Member Author

It's ok, I'll explain it more though, form submissions via button click automatically invoke form validation in an HTML view but keyboard shortcuts are tied to menu items and do not invoke HTML view form validation. So the form validation is not deployed when the user users keyboard shortcuts.

If we tied keyboard shortcuts to buttons, then we would be using access keys, which users generally hate, and which would be counter to wanting to have a native app feel. So we do want to keep them in the menu. But then the validation step is missing. So the next feedback the user gets comes from the engine, and those error messages are not very friendly in the context of the UI.

So we need it to be a two-step process. It can be contained within the UI and does not have to involve the engine, unless of course the engine would be a good place for it, such as the two-step braille configuration.

I deleted my earlier comment as in hindsight it was not helpful to this thread. Sorry.

@bertfrees
Copy link
Member

Ok, clear!

@bertfrees
Copy link
Member

Besides the validation API for input files, there was also the idea of a validation API for options. I don't know whether for this use case the engine is the right place for it though, but we can keep it in mind. By the way I guess it doesn't even need to be separate API calls, perhaps it could be integrated in the job submission API: instead of the generic error, a specific error report could be returned in case of validation issues (and in case of no validation issues you'd get the same behavior as before). That way it would stay a single-step process. Not sure if it's RESTful though.

@marisademeglio
Copy link
Member Author

We could have a /validation endpoint that would return if a job's inputs/options were valid. That would be easier on the UI than lumping it in with regular job submission, though I guess we could rework that to make it fit.

It still might be slow though to do it in the engine, at least until we move from polling to push updates.

@bertfrees
Copy link
Member

Yes, it might indeed be a bit slower than doing it entirely in the UI.

Although, I was very pleasantly surprised about the responsiveness of the two-step braille configuration.

@marisademeglio
Copy link
Member Author

With an approach for #244 like what we discussed here
https://daisy-dev.slack.com/archives/C064GB8U9/p1728313876488069

we can take advantage of HTML form-side validation, which should be sufficient, especially if we can extend it in the future for custom datatypes.

@marisademeglio
Copy link
Member Author

We decided to use HTML form-side validation after all (it's sufficient), and found a way to trigger it from the menu, so the reason for creating this issue is now kind of gone. So I'm closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants