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

After Random characters in autosuggest field for categories, you'll get directly the XHR response #9884

Closed
forgive38 opened this issue Apr 12, 2024 · 9 comments
Assignees
Labels
Try Me This issue might be good for a new contributor. Can you help us? UI/UX Issues affecting the user interface/user experience
Milestone

Comments

@forgive38
Copy link
Contributor

Describe the bug
During th submission process, when you have more than 10 categories, a autosuggest field is used. And if you enter some random characters in and hit Enter, you'll get the result of XHR request.

To Reproduce

  1. Go to testdrive 3.4
  2. enable categories, add more than 10 categories
  3. make a new submission and in For the Editors tab, try it
  4. See error

What application are you using?
OJS 3.4 testdrive

@asmecher asmecher added the Try Me This issue might be good for a new contributor. Can you help us? label May 9, 2024
@asmecher asmecher added this to the 3.4.0-6 milestone May 9, 2024
@jardakotesovec jardakotesovec self-assigned this Jul 15, 2024
@jardakotesovec jardakotesovec added the UI/UX Issues affecting the user interface/user experience label Jul 15, 2024
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Jul 15, 2024
@jardakotesovec
Copy link
Contributor

jardakotesovec commented Jul 15, 2024

This was interesting to track down. Apparently there is some default browser behaviour specifically on form with one field. So hitting enter in that one field was automatically submitting the form, which was causing to display the endpoint where the form was pointing to.

But additionally if there is form with one input and some button - it will hit the button automatically?! So additional to the report, what was happening is that if you would enter valid category and than you would type something that does not exist and hit enter - it clicked on the 'remove' button of the previous category just based on that there is SOME button in the form.. Can't be more confusing than that.

What seems to be effective way to prevent it is just discard such enter event on the form level. I did test it with chrome, safari and firefox, so hopefully its good fix.

@jonasraoni Can you please do the code review?

Will check whether same problem is on 3.3 and main branch.

3.4.0 PR
pkp/ui-library#381

jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Jul 15, 2024
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Jul 15, 2024
@jardakotesovec
Copy link
Contributor

jardakotesovec commented Jul 15, 2024

3.3 is using legacy form with checkboxes, so not relevant in that particular use case, but it can be problem in other vue.js form which would have just one FieldText for example

Therefore adding same improvement for 3.3 pkp/ui-library#383

And same thing for main - its not problem in this particular scenario, because the autocomplete component from headless-ui does not let propagate enter event. But its misbehaving for example with simple FieldText.

After the code review I will also cherry pick it to the main branch.

@jonasraoni
Copy link
Contributor

Yeah, this behavior is old. I agree with the fix 😁

The worst was the backspace behaving as a back button 🥲

jardakotesovec added a commit to pkp/ui-library that referenced this issue Jul 17, 2024
jardakotesovec added a commit to pkp/ui-library that referenced this issue Jul 17, 2024
jardakotesovec added a commit to pkp/ui-library that referenced this issue Jul 17, 2024
@jardakotesovec
Copy link
Contributor

Merged to 3.3 and 3.4 and cherry-picked to main. Thanks @jonasraoni

@bozana
Copy link
Collaborator

bozana commented Oct 22, 2024

After this change it is not possible to enter a new line in the field References.

Removing @keydown.enter.prevent="" apparently fixes it.

Note:

... even the original issue why I added this was sneaky behavior when there is just one field in the form...
But that hitting enter in textarea will get suppressed if I am ignoring this keydown on form element - I don't see how someone could anticipate such behavior...

... to solve this in the context of this original issue report... making sure that both scenarios works...

@bozana bozana modified the milestones: 3.4.0-6, 3.3.0-20, 3.4.0-8 Oct 22, 2024
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 24, 2024
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 24, 2024
@jardakotesovec
Copy link
Contributor

jardakotesovec commented Oct 24, 2024

So this is alternative I came up with. Reasoning behind this is that we don't rely on submit event at all. We use normal button to submit form, which triggers our custom vue.js event. Therefore just preventing default behaviour on submit should be safe and it prevents the scenario, when there is one input and no button in the form to automatically triggers submit event and therefore open the 'action' url.

But thats not enough. As mentioned above - if there is one input and some button - it will automatically click on that button - no matter what that button does. So for example in categories autocomplete it was removing the categories on entry, because the remove icons are buttons.

Therefore also added <input type="submit" value="i9884" style="display: none" />. I can't think of any side effect that this might cause and now its the button which gets automatically clicked on when there is just one input. Which triggets the submit event on form and that gets ignored with the @submit.prevent="() => {}".

And this new solution is not interfering with hitting enter - which cause that issue in textarea that @bozana mentioned.

I did various testing across browsers and versions. So hopefully this is solid solution.

I will need two approvals for 3.3 and 3.4, can I ask @blesildaramirez and @ewhanson ?

main:
ui-library: pkp/ui-library#432
ojs (tests): pkp/ojs#4485 pkp/ojs#4491

3.4:
ui-library: pkp/ui-library#433
ojs (tests): pkp/ojs#4486 pkp/ojs#4492

3.3
ui-library: pkp/ui-library#434
ojs (tests): pkp/ojs#4487 pkp/ojs#4493

Note from Blessie: OJS test links have been changed due to correction on commit message, to point to correct branch names in ui-library. The ui-library branches are from Jarda's original branch, just making sure the tests in OJS points to intended changes in ui-library for accurate testing runs.

jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 24, 2024
@jardakotesovec
Copy link
Contributor

Test for main are failing.. let me check whether its all rebased

jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 24, 2024
@jardakotesovec
Copy link
Contributor

All tests are passing now.

jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 25, 2024
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 25, 2024
jardakotesovec added a commit to jardakotesovec/ui-library that referenced this issue Oct 25, 2024
blesildaramirez pushed a commit to blesildaramirez/ui-library that referenced this issue Oct 28, 2024
blesildaramirez pushed a commit to blesildaramirez/ui-library that referenced this issue Oct 28, 2024
blesildaramirez added a commit to blesildaramirez/ojs that referenced this issue Oct 28, 2024
blesildaramirez added a commit to blesildaramirez/ojs that referenced this issue Oct 28, 2024
blesildaramirez pushed a commit to pkp/ui-library that referenced this issue Oct 29, 2024
#432)

* pkp/pkp-lib#9884 Second attempt to override default behaviour for one input forms

* pkp/pkp-lib#9884 Add explanatory comment to dummy submit input
asmecher pushed a commit to pkp/ui-library that referenced this issue Oct 30, 2024
#434)

* pkp/pkp-lib#9884 Second attempt to override default behaviour for one input

* pkp/pkp-lib#9884 Add explanatory comment to dummy submit input
asmecher pushed a commit to pkp/ui-library that referenced this issue Oct 30, 2024
#433)

* pkp/pkp-lib#9884 Second attempt to override default behaviour for one input

* pkp/pkp-lib#9884 Add explanatory comment to dummy submit input
@asmecher
Copy link
Member

All merged (except submodules which will be handled separately), thanks!

@asmecher asmecher modified the milestones: 3.4.0-8, 3.3.0-20 Oct 30, 2024
ipula pushed a commit to ipula/ui-library that referenced this issue Nov 1, 2024
pkp#432)

* pkp/pkp-lib#9884 Second attempt to override default behaviour for one input forms

* pkp/pkp-lib#9884 Add explanatory comment to dummy submit input
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Try Me This issue might be good for a new contributor. Can you help us? UI/UX Issues affecting the user interface/user experience
Projects
None yet
Development

No branches or pull requests

5 participants