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

Fix keyboard navigation + cron job notification #1953

Merged
merged 2 commits into from
Oct 23, 2022

Conversation

rhyst
Copy link
Contributor

@rhyst rhyst commented Oct 22, 2022

Keyboard navigation through items was not working correctly in NC25 and highlighting active item on scroll was not working in NC25, NC24, NC23 (I think) so I have attempted to fix this.

The cronjob warning notification layout was messed up on NC25 as well so also attempted a quick fix for that.

Also tidied up the version number stuff a bit. I feel like the Nextcloud JS globals (the window.OC object) should have that info but I couldn't find it anywhere 🤷. Dumping the version number from PHP into a data attribute feels wrong 🤠

Copy link
Member

@Grotax Grotax left a comment

Choose a reason for hiding this comment

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

during build gulp fails

> node node_modules/gulp-cli/bin/gulp.js

[07:32:26] Using gulpfile /workspaces/news/js/gulpfile.js
[07:32:26] Starting 'default'...
[07:32:26] Starting 'lint'...
gui/KeyboardShortcuts.js: line 234, col 38, 'scrollArea' is defined but never used.

1 error
[07:32:27] 'lint' errored after 559 ms
[07:32:27] Error in plugin "gulp-jshint"
Message:
    JSHint failed for: gui/KeyboardShortcuts.js
Details:
    domainEmitter: [object Object]
    domainThrown: false

[07:32:27] 'default' errored after 562 ms
make[1]: *** [Makefile:88: npm] Error 1
make[1]: Leaving directory '/workspaces/news'
make: *** [Makefile:69: build] Error 2

@rhyst
Copy link
Contributor Author

rhyst commented Oct 23, 2022

Oops, should be fixed now. The effect of removing a console.log at the last minute 😄

Signed-off-by: Rhys Tyers <mail@rhy.si>
Signed-off-by: Rhys Tyers <mail@rhy.si>
@rhyst
Copy link
Contributor Author

rhyst commented Oct 23, 2022

Sorry I managed to completely mess up that rebase so most of my changes weren't in. Pushed again, will just confirm this still does what I think it does.

@rhyst
Copy link
Contributor Author

rhyst commented Oct 23, 2022

I think its good 😬

@Grotax Grotax merged commit 0822765 into nextcloud:master Oct 23, 2022
Grotax added a commit that referenced this pull request Oct 23, 2022
Fixed
- Fixed various keyboard navigation issues (#1953)
- Fix cron job warning notification layout on NC25 (#1953)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Oct 23, 2022
Grotax added a commit that referenced this pull request Oct 23, 2022
Fixed
- Fixed various keyboard navigation issues (#1953)
- Fix cron job warning notification layout on NC25 (#1953)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants