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.

Sign up for GitHub

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

Jump to bottom

Replace Hotkey library #275

Merged
merged 13 commits into from
May 23, 2024
Merged

Replace Hotkey library #275

merged 13 commits into from
May 23, 2024

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Jan 22, 2024

Resolves #203

Replaces react-hotkeys with react-hotkeys-hooks.
The latter is maintained, easier in use and already used in other Opencast frontend projects.

Also makes a small change to the hotkey cheat sheet, where hotkeys that can currently not be used are
displayed in a lower opacity.

@Arnei
Replace Hotkey library
34ace2f 
Replaces react-hotkeys with react-hotkeys-hooks.
The latter is maintained, easier in use and already
used in other Opencast frontend projects.

Also makes a small change to the hotkey cheat sheet,
where hotkeys that can currently not be used are
displayed in a lower opacity.
@Arnei Arnei added type:enhancement New feature or request type:dependencies Pull requests that update a dependency file labels Jan 22, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 4, 2024

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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 3, 2024

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

@ziegenberg
Copy link
Member

The browser console is complaining:

i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.J j
i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.M m
i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.R r
i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.H h
i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.E e
i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.S s
i18next::translator: missingKey en-GB translation HOTKEYS.KEYS.N n

Should we add the missing translations to en-US (see #406, why en-US and not en-GB)?

ziegenberg
ziegenberg reviewed May 22, 2024
View reviewed changes
Copy link
Member

@ziegenberg ziegenberg left a comment

Choose a reason for hiding this comment

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

I went ahead and clicked through, and it worked. I have a few minor comments, but otherwise, looking good!

app/src/configs/hotkeysConfig.ts Outdated Show resolved Hide resolved
app/src/components/Header.tsx Outdated Show resolved Hide resolved
app/src/components/events/Events.tsx Outdated Show resolved Hide resolved
@ziegenberg ziegenberg mentioned this pull request May 22, 2024
Closed
@ziegenberg
Copy link
Member

This probably also "solves" #73, right?

Arnei added 3 commits May 23, 2024 09:15
@Arnei
Remove unused translations strings for hotkeys
77927cb 
WIth no theodul player and no editor
integrated with the admin ui, there are also
no more hotkeys for those respective tools.
Hence their translations can go as well.
@Arnei
Formatting
68b8af1
@Arnei
Add translations for letter hotkeys
5b04561 
React complains about missing keys. This
is an easier fix than trying to discriminate
between keys that should be translated and
keys that should not be translated.
@Arnei
Copy link
Member Author

Arnei commented May 23, 2024

This probably also "solves" #73, right?

It will be "fixed" in the way you described in #73, yes.

@ziegenberg ziegenberg self-requested a review May 23, 2024 07:59
@ziegenberg
Copy link
Member

Looks good to me 😊

@github-actions GitHub Actions
Copy link
Contributor

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

@ziegenberg ziegenberg merged commit 25b79b3 into opencast:main May 23, 2024
2 checks passed
@ziegenberg ziegenberg self-assigned this May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ziegenberg ziegenberg ziegenberg approved these changes

Assignees

@ziegenberg ziegenberg

Labels
type:dependencies Pull requests that update a dependency file type:enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Get rid of react-hotkeys
2 participants
@Arnei @ziegenberg