jenkins icon indicating copy to clipboard operation
jenkins copied to clipboard

[JENKINS-71699] ensure cancel action is executed on "Enter"

Open mawinter69 opened this issue 1 year ago • 9 comments

When making the cancel button in a dialog the selected element pressing "Enter" or "Space" should trigger the buttons action and not the default Ok button.

See JENKINS-71699.

Testing done

Selecting the "Cancel" button test

  • Create a new api token
  • click on the delete icon
  • press tab until "Cancel" button is select
  • press "Enter" -> api token is not deleted

Selecting the "Ok" button test

  • Create a new api token
  • click on the delete icon
  • press tab until "Ok" button is selected
  • press "Enter" -> api token is deleted

Default behaviour test:

  • Create a new api token
  • click on the delete icon
  • press "Enter" -> api token is deleted

Proposed changelog entries

  • [JENKINS-71699] ensure cancel action is executed on "Enter"

Proposed upgrade guidelines

N/A

### Submitter checklist
- [x] The Jira issue, if it exists, is well-described.
- [ ] The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see [examples](https://github.com/jenkins-infra/jenkins.io/blob/master/content/_data/changelogs/weekly.yml)). Fill in the **Proposed upgrade guidelines** section only if there are breaking changes or changes that may require extra steps from users during upgrade.
- [ ] There is automated testing or an explanation as to why this change has no tests.
- [ ] New public classes, fields, and methods are annotated with `@Restricted` or have `@since TODO` Javadocs, as appropriate.
- [ ] New deprecations are annotated with `@Deprecated(since = "TODO")` or `@Deprecated(forRemoval = true, since = "TODO")`, if applicable.
- [ ] New or substantially changed JavaScript is not defined inline and does not call `eval` to ease future introduction of Content Security Policy (CSP) directives (see [documentation](https://www.jenkins.io/doc/developer/security/csp/)).
- [ ] For dependency updates, there are links to external changelogs and, if possible, full differentials.
- [ ] For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

### Maintainer checklist
- [ ] There are at least two (2) approvals for the pull request and no outstanding requests for change.
- [ ] Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
- [ ] Changelog entries in the pull request title and/or **Proposed changelog entries** are accurate, human-readable, and in the imperative mood.
- [ ] Proper changelog labels are set so that the changelog can be generated automatically.
- [ ] If the change needs additional upgrade steps from users, the `upgrade-guide-needed` label is set and there is a **Proposed upgrade guidelines** section in the pull request title (see [example](https://github.com/jenkinsci/jenkins/pull/4387)).
- [ ] If it would make sense to backport the change to LTS, a Jira issue must exist, be a _Bug_ or _Improvement_, and be labeled as `lts-candidate` to be considered (see [query](https://issues.jenkins.io/issues/?filter=12146)).

mawinter69 avatar Jul 24 '23 16:07 mawinter69

this is an improvement on what we have and fixes the issue.

There still seems to be some niggles though.

  1. tab order is strange - goes from right to left not left to right (Yes -> Cancel)
  2. the default button does not have initial focus (unlike the native dialog), there may be some accessibility issues switching the focus to a dialog and then having to remove it.

jtnord avatar Jul 26 '23 11:07 jtnord

  1. tab order is strange - goes from right to left not left to right (Yes -> Cancel)

That is due to the flexbox model going right to left, though the "OK" button is defined first. Not sure if this is really an issue, it makes the "OK" the first thing that gets selected after pressing tab once in a dialog.

  1. the default button does not have initial focus (unlike the native dialog), there may be some accessibility issues switching the focus to a dialog and then having to remove it.

The "OK" button is the default action when you press "Enter" in a dialog without selecting anything. So it is not absolutely necessary to set the focus to it I think. And when you have a prompt dialog the focus is set to the input field and pressing "Enter" will accept the input, similar to the browser built-in.

mawinter69 avatar Jul 26 '23 11:07 mawinter69

On Windows when in a native browser dialog, the selected button is pressed with "Space" and "Enter". This might be different on MacOS, I don't know. Also I in Chrome and Firefox on windows, when you select a

mawinter69 avatar Jul 26 '23 12:07 mawinter69

This might be different on MacOS, I don't know.

It is, that's my point. Enter = Default action, Space = selected action. On this dialog, the keys do different things:

Screenshot 2023-07-26 at 14 59 54

Here they do the same thing:

Screenshot 2023-07-26 at 15 00 58

daniel-beck avatar Jul 26 '23 13:07 daniel-beck

Then either we make the behaviour inconsistent for windows users or for mac users. Or we detect the OS (might not be 100% reliable) but then it might be confusing for people that use both mac and windows. Before this fix that @jtnord requested the behaviour was more the mac like, so selecting "Cancel" and pressing "Enter" would trigger the OK action. But this behaviour is in contrast to all other buttons in a browser where when selected it doesn't matter if you use "Enter" or "Space" it is triggering the selected button.

For most user it probably doesn't matter as they use the mouse.

mawinter69 avatar Jul 26 '23 13:07 mawinter69

Please take a moment and address the merge conflicts of your pull request. Thanks!

github-actions[bot] avatar Jul 27 '23 07:07 github-actions[bot]

Then either we make the behaviour inconsistent for windows users or for mac users. Or we detect the OS (might not be 100% reliable) but then it might be confusing for people that use both mac and windows. Before this fix that @jtnord requested the behaviour was more the mac like, so selecting "Cancel" and pressing "Enter" would trigger the OK action.

That was not my request, I was asking that Enter would trigger the selected button. Are we misunderstanding each other?

But this behaviour is in contrast to all other buttons in a browser where when selected it doesn't matter if you use "Enter" or "Space" it is triggering the selected button.

this behaviour being the one I asked for, or the one in this PR?

For most user it probably doesn't matter as they use the mouse.

or revert and use the native browser functionality ;-)

Enter is not normally "default action" on web interfaces - it is normally form submit. o me a dialog is not a form, and when a "form" has 2 submit buttons it is impossible to know which is the default without some form of other styling. A big red button implies destructive behaviour but not default behaviour to me - dunno - maybe remove the "enter" handling altogether - but that seems strange also?

@cristina-pizzagalli any thoughts on this?

jtnord avatar Jul 27 '23 10:07 jtnord

Then either we make the behaviour inconsistent for windows users or for mac users. Or we detect the OS (might not be 100% reliable) but then it might be confusing for people that use both mac and windows. Before this fix that @jtnord requested the behaviour was more the mac like, so selecting "Cancel" and pressing "Enter" would trigger the OK action.

That was not my request, I was asking that Enter would trigger the selected button. Are we misunderstanding each other?

No, I understood it like that and this change will ensure when the "Cancel" button is selected and "Enter" is pressed then the "Cancel" action will be performed.

But this behaviour is in contrast to all other buttons in a browser where when selected it doesn't matter if you use "Enter" or "Space" it is triggering the selected button.

this behaviour being the one I asked for, or the one in this PR?

This PR will ensure the behaviour you asked for which is then the same as for other browser buttons.

For most user it probably doesn't matter as they use the mouse.

or revert and use the native browser functionality ;-)

Enter is not normally "default action" on web interfaces - it is normally form submit. o me a dialog is not a form, and when a "form" has 2 submit buttons it is impossible to know which is the default without some form of other styling. A big red button implies destructive behaviour but not default behaviour to me - dunno - maybe remove the "enter" handling altogether - but that seems strange also?

@cristina-pizzagalli any thoughts on this?

The native browser confirm dialogs put the focus on the OK button, so you can directly press "Enter" to confirm the action. And when you have a prompt where you also ask for input it is the same. After entering some text and the focus is still in the input field, just pressing "Enter" is like pressing the "Ok" button. I tried to reproduce this behaviour for both scenarios.

mawinter69 avatar Jul 27 '23 11:07 mawinter69