Skip to content

Conversation

@EnsiyehE
Copy link
Contributor

Hello all,

as stated in the problem #984 , there was no possibility like the old version of admin user interface of open cast to navigate between different steps like ( edit event modal ) (eg, metadata to publications ) ,

by this PR i tried to suggest the solution which i would appreciate your view and opinions , if i am in the currect track or even close to it that i can continue my solution to other wizard modals inside the Admin UI ,

now it is possible to move forward whenever the focus is not on the input type fields by pressing: alt + Enter and back ward : alt + Backspace , to test this PR please first open the edit window modal , and change the focus by tabbing from the input field to the next one and then use the hotkeys as above i have mentioned them

thank you all for your Time
Ensiyeh

@github-actions
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-1447

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-1447

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.

@github-actions
Copy link
Contributor

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

@pascalseeland pascalseeland added the type:enhancement New feature or request label Sep 30, 2025
Copy link
Member

@gregorydlogan gregorydlogan left a comment

Choose a reason for hiding this comment

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

Works, nitpicking about the shortcut descriptions

@github-actions
Copy link
Contributor

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.

Seems to work fine. The linked issue requested the next/previous hotkeys for the create modals though, not for the details modals. Not that having them in the details modal would be bad, but they're more useful in the create modals where users will need to move between adjacent tabs more often.

}
};
const wizardTabs = tabs.filter(tab =>
!tab.hidden && tab.page !== EventDetailsPage.Tobira && tab.page !== EventDetailsPage.Statistics,
Copy link
Member

Choose a reason for hiding this comment

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

Why are we explicitly avoiding the Tobira and Statistics tab?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello

As I see it on my machine (please correct me if I’m wrong), these elements are not part of the Event Details tab, so I couldn’t find a reason for them to be included in the logic. This was just an experiment for this specific modal.

My goal was actually to generalize the behavior, and I mentioned this in the issue to you and Lars, who opened it, but I unfortunately didn’t receive any feedback.

I believe that trying to generalize this inside the parent element could lead to bigger issues, because the Modal component handles different types of modals — such as confirmations and others — which have different types and behaviors.

eventdetails tab names

Copy link
Member

@Arnei Arnei Nov 13, 2025

Choose a reason for hiding this comment

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

As I see it on my machine (please correct me if I’m wrong), these elements are not part of the Event Details tab, so I couldn’t find a reason for them to be included in the logic. This was just an experiment for this specific modal.

These elements are part of the event details, but do not show per default. Much like the extended metadata tab for example.They would appear if their hidden condition evaluates to false.

But if this was just an experiment and is bound to change in the future, no need to sweat over it.

Copy link
Contributor Author

@EnsiyehE EnsiyehE Nov 13, 2025

Choose a reason for hiding this comment

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

These elements are part of the event details, but do not show per default. Much like the extended metadata tab for example.They would appear if their hidden condition evaluates to true.

But if this was just an experiment and is bound to change in the future, no need to sweat over it.

Thanks for the explanation! I’ve made the tabs that are not hidden at the moment as the wizardTabs 😊

availableHotkeys.general.NEXT_PANEL.sequence,
event => {
const target = event.target as HTMLElement;
if (!["INPUT", "TEXTAREA"].includes(target.tagName)) {
Copy link
Member

Choose a reason for hiding this comment

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

I believe we don't need to explicitly check for "INPUT" or "TEXTAREA" here, the useHotkeys hook should do that per default.

Copy link
Contributor Author

@EnsiyehE EnsiyehE Nov 12, 2025

Choose a reason for hiding this comment

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

The reason is ::: the hotkey doesn’t trigger while the first input is autofocused is due to how react-hotkeys-hook handles keyboard events by default. Keyboard events are sent to the element that currently has focus. By default, this library ignores events coming from input, textarea, or other editable elements to avoid interfering with typing.

You can see this explained in the docs here ::: https://react-hotkeys-hook.vercel.app/docs/api/use-hotkeys
example

As you previously mentioned in the underlying issue, the same behavior occurs, and this could have been a suggestion for a reasonable solution for the issue:

issue explainations

Copy link
Member

@Arnei Arnei Nov 13, 2025

Choose a reason for hiding this comment

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

Oh so we want the hotkeys to be able to trigger while an input or textarea element is in focus. Makes sense to me. Then you have to invert your if condition I think. Or make use of enableOnFormTags, that is probably cleaner.

Copy link
Contributor Author

@EnsiyehE EnsiyehE Nov 13, 2025

Choose a reason for hiding this comment

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

I think there was a misunderstanding on my side. The goal of this condition was to allow the hotkeys to trigger even when an input or textarea is focused. We used this if (!["INPUT", "TEXTAREA"].includes(target.tagName)) check as a practical workaround because, due to how the hotkey listener is attached globally, event.target is observed as the body element even when the input is focused. This makes the hotkeys fire correctly in that context. Essentially, the condition ensures that the hotkeys work as intended inside input or textarea fields, despite the event target not being the input itself.

! makes it fire inside input fields — because target.tagName is "BODY" even when an input is focused.

Copy link
Member

Choose a reason for hiding this comment

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

! makes it fire inside input fields

But it does not. If I open the event details, focus an input field and then press "Alt + Enter", the input field simply looses focus like if I had just pressed "Enter".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imagine :::

If we removed the condition entirely, the initial focus on the first input field (which is always set by default) would prevent us from moving forward or backward between tabs or steps. Without this feature, the hotkeys simply wouldn’t work when the input is focused.

As I mentioned before, the first time you press Esc, it would remove the focus trap from the window completely( could you try it please on your develop branch now on the same window ?? eventdetails ) . I have already solved that issue, but it should be tracked separately in another ticket.( you mentioned in one of the discussions )

This condition is necessary to allow navigation( by keyboard )) while respecting the default behavior of focus on input fields.

Copy link
Member

Choose a reason for hiding this comment

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

Are you expecting that the user should be able to trigger the hotkey while an input or textarea is focused?

That is the behaviour I think is desirable. I am not saying that your code does this (it does not), but I think that is the behaviour we would ultimately want.

In other words, if the focus is on an input or textarea, we cannot trigger the hotkey — this is not something we can change.

I think we can. This should do the trick:

	useHotkeys(
		availableHotkeys.general.NEXT_PANEL.sequence,
		event => {
				goNextStep();
		},
		{ enableOnFormTags: ["INPUT", "TEXTAREA"] },
		[goNextStep],
	);

Copy link
Member

Choose a reason for hiding this comment

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

If we removed the condition entirely, the initial focus on the first input field (which is always set by default) would prevent us from moving forward or backward between tabs or steps. Without this feature, the hotkeys simply wouldn’t work when the input is focused.

Previously you claimed that we cannot make hotkeys work while the input element is focused. Now you seem to claim that due to "the condition", the hotkeys work while the input element is focused. That sounds like a contradiction to me, so I probably still don't understand what you are trying to explain. What exactly do you expect "the condition" to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with all due respect :

I prefer to use your code for now to avoid further discussions, while I focus on other issues. I will revisit this in the coming days . If it doesn’t work, I will try to explain my intent again. As I’ve already mentioned in the explanation of the issue:

Currently, it is possible to move forward whenever the focus is not on an input field by pressing Alt + Enter (forward) and Alt + Backspace (backward). To test this PR: first open the edit window modal, then tab from one input field to the next, and use the hotkeys as described above.

When I said we can solve it “whenever we are focused on the input,” I meant exactly this scenario.

If we removed the condition entirely, the initial focus on the first input field (which is set by default) would prevent us from moving forward or backward between tabs or steps. Without this condition, the hotkeys wouldn’t work when the input is focused.

As I mentioned before, pressing Esc the first time currently removes the focus trap from the window completely. Could you please try this on your develop branch using the same window (e.g., EventDetails)? I have already solved that issue, but it should be tracked separately in another ticket, as you noted in one of our discussions.

This condition is necessary to allow keyboard navigation while still respecting the default behavior of focusing on input fields.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer to use your code for now to avoid further discussions, while I focus on other issues

Understandable, good luck with your endeavours.

Currently, it is possible to move forward whenever the focus is not on an input field by pressing Alt + Enter (forward) and Alt + Backspace (backward).

I can confirm that.

To test this PR: first open the edit window modal, then tab from one input field to the next, and use the hotkeys as described above.

If I for example open the event details modal, then tab from title to subject, then press the "Next" hotkey, it will not take me to the next tab. Only after pressing the "Next" hotkey a second time will it take me to the next tab. Is that what you expect to happen?

If we removed the condition entirely, the initial focus on the first input field (which is set by default) would prevent us from moving forward or backward between tabs or steps. Without this condition, the hotkeys wouldn’t work when the input is focused.

From my testing, removing the condition (we are talking about if (!["INPUT", "TEXTAREA"].includes(target.tagName)), right?) does not have any effect, the behaviour stays the same.

As I mentioned before, pressing Esc the first time currently removes the focus trap from the window completely. Could you please try this on your develop branch using the same window (e.g., EventDetails)?

With your PR, the focus trap stays intact as far as I can tell. Pressing ESC while inside an input field and then pressing tab will put the focus on the close-modal button. And I cannot tab outside the modal.

return ReactDOM.createPortal(
isOpen &&
<FocusTrap>
<FocusTrap focusTrapOptions={{ escapeDeactivates: false }}>
Copy link
Member

Choose a reason for hiding this comment

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

This seems unrelated to the issue at hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello,

As I mentioned earlier in the issue: when a user submits a title and presses Enter (to temporarily remove the autofocus from the first input field in the Add Event or Edit Event modal), or presses Escape, we sometimes lose the focus trap of the modal.

This means the focus shifts to browser elements instead of staying within the modal — which I believe is not a good UX experience.

I’ve now fixed this issue with my latest code update. When you press Escape for the first time, the modal retains its focus trap as intended. On the second press, it behaves as expected and closes the modal.

Previously, as I mentioned in the issue, pressing Escape once would immediately break the focus trap — which I think was a poor UX experience.

Copy link
Member

Choose a reason for hiding this comment

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

That may all be true, but still has nothing to do with adding next/previous hotkeys. Please submit this as a separate PR. Having different features as separate PRs has several benefits:

  • It is easier for reviewers to tell which code change belongs to which feature, speeding up the review process.
  • Having separate features as separate commits leads to a cleaner commit history.
  • If the first feature is approved but the second feature is controversial, the first feature is not blocked by the second feature.

Comment on lines +19 to +29
// Store the last blurred/focused element
const lastFocusedRef = useRef<HTMLElement | null>(null);
const handleKeyDown = (event: React.KeyboardEvent<HTMLDivElement>) => {
const active = document.activeElement as HTMLElement | null;
if (event.key === "Escape") {
if (active && event.currentTarget.contains(active)) {
lastFocusedRef.current = active; // store the last focused element
active.blur();
}
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This also seems unrelated to the issue at hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have mentioned the reason on the above discussion already

bests
Ensiyeh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants