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 keyboard keys to remove focus on input fields #733

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

dennis531
Copy link
Contributor

Adds a keyboard event handler to remove the focus from input elements when the esc or enter key are pressed. This pull request is based on #643 (comment).

Close #643

Copy link
Contributor

github-actions bot commented Jun 24, 2024

This pull request is deployed at test.admin-interface.opencast.org/733/2024-07-11_12-13-21/ .
It might take a few minutes for it to become available.

Copy link
Contributor

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-733

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-733

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

@dennis531 dennis531 added type:accessibility This would help impaired users type:usability Improves the UX labels Jun 24, 2024
@rlucke
Copy link
Contributor

rlucke commented Jun 26, 2024

I don't understand the point of this pull request. There is already a key handler in RenderField.

// Handle key down event and check if pressed key leads to leaving edit mode
	const handleKeyDown = (event: React.KeyboardEvent, type: MetadataField["type"]) => {
		const { key } = event;
		// keys pressable for leaving edit mode
		const keys = ["Escape", "Tab", "Enter"];

		if (type !== "textarea" && keys.indexOf(key) > -1) {
			setEditMode(false);
		}
	};

And you can already use "Escape", "Tab", "Enter" to leave the input field.

@dennis531
Copy link
Contributor Author

I don't understand the point of this pull request. There is already a key handler in RenderField.

This is true, but it doesn't apply to all field components, as not all of them have a component property. In these cases, a simple HTML input will be used, which does not support Enter and ESC keys (at least in Firefox). For example, in the create user dialog, the RenderField or RenderMutlifield is not passed as component prop.

To avoid redundant code, I created this wrapper that adds this key down handler to all Field components. If a component prop with value such as RenderField is passed, this handler will be ignored in the RenderField component.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@lkiesow
Copy link
Member

lkiesow commented Jul 2, 2024

Then, do we still need the code in RenderField at all?

@dennis531
Copy link
Contributor Author

Then, do we still need the code in RenderField at all?

Yes, because the handler in RenderField manages the specific state of this component, which is different from the basic HTML input logik.

Copy link
Contributor

github-actions bot commented Jul 5, 2024

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

While I can see developers accidentally importing the wrong Field component in the future, this does seem like the most reasonable solution to the given problem. Works and can be merged after the current merge conflicts are fixed imo.

@Arnei
Copy link
Member

Arnei commented Jul 11, 2024

For stuff like the "Add User" oder "Create group" modals, it might be better if the enter key worked more like the tab key in that pressing it would automatically focus the next field. Not something that needs to be changed by this PR, certainly, I'm just musing here. I also have not thought about all UX or a11y implications yet.

@Arnei Arnei merged commit 401825b into opencast:main Jul 12, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:accessibility This would help impaired users type:usability Improves the UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some input fields are missing keyboard accessibility/convenience
4 participants