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 search shortcut support #3257

Merged
merged 4 commits into from
Apr 9, 2020

Conversation

marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Apr 3, 2020

@marcoambrosini
Copy link
Member Author

/backport to stable18

@marcoambrosini marcoambrosini self-assigned this Apr 3, 2020
@marcoambrosini marcoambrosini added this to the 💙 Next Minor (18) milestone Apr 3, 2020
@marcoambrosini marcoambrosini force-pushed the feature/3195/add-search-shortcut-support branch from ce36de7 to 036776c Compare April 3, 2020 13:18
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
@marcoambrosini marcoambrosini marked this pull request as ready for review April 3, 2020 13:19
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
@skjnldsv
Copy link
Member

skjnldsv commented Apr 3, 2020

/backport to stable18

Not so sure for this anymore though. Let's make sure there is no regressions at all :)

@nickvergessen
Copy link
Member

Setting "to develop" as dependency is still open

@marcoambrosini
Copy link
Member Author

Setting "to develop" as dependency is still open

Can be merged anyway imo

@nickvergessen
Copy link
Member

But we need to bump the vue library after it was merged?

@skjnldsv
Copy link
Member

skjnldsv commented Apr 7, 2020

But we need to bump the vue library after it was merged?

the only thing missing here would be the auto-open of the sidebar, the shortcut and focus will still works

@marcoambrosini
Copy link
Member Author

yep

src/App.vue Outdated Show resolved Hide resolved
@@ -299,33 +300,4 @@ export default {
border-bottom: 1px solid var(--color-border-dark);
}

.navigation {
Copy link
Member

Choose a reason for hiding this comment

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

I strongly suggest you do a tour of your code to check where are the other locations you did something similar and nuke it 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

lol, ok :)

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Clean!

Co-Authored-By: John Molakvoæ <skjnldsv@users.noreply.github.com>
@marcoambrosini marcoambrosini merged commit 35dcc53 into master Apr 9, 2020
@marcoambrosini marcoambrosini deleted the feature/3195/add-search-shortcut-support branch April 9, 2020 07:29
@backportbot-nextcloud
Copy link

The backport to stable18 failed. Please do this backport manually.

1 similar comment
@backportbot-nextcloud
Copy link

The backport to stable18 failed. Please do this backport manually.

@nickvergessen
Copy link
Member

Needs a manual backport @ma12-co
Can you do that?

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

Successfully merging this pull request may close these issues.

3 participants