Skip to content

Conversation

@marcelklehr
Copy link
Member

@marcelklehr marcelklehr commented Aug 1, 2020

see nextcloud/server#21971

  • deal with nc<20 compatibility
  • deal with in-app search
  • implement a proper route for individual bookmarks

@marcelklehr marcelklehr marked this pull request as ready for review August 2, 2020 09:01
@skjnldsv
Copy link
Member

skjnldsv commented Aug 2, 2020

Bookmarks has its own search currently. Any recommendations on how this should be handled?

Inner filtering can be done on the top right search design, like Talk.
For the global search you can rely on the Unified search.

But regarding bookmarks, I'm wondering if this shouldn't be done with Unified search only, no?
You only have one type of items to search in the end :)

Comment on lines 224 to +286
.breadcrumbs {
padding: 0 8px 0 44px;
display: flex;
position: absolute;
z-index: 100;
background: var(--color-main-background-translucent);
left: 0;
right: 0;
top: 0;
padding: 0 8px 0 44px;
display: flex;
position: absolute;
z-index: 100;
background: var(--color-main-background-translucent);
left: 0;
right: 0;
top: 0;
}
.breadcrumbs + * {
margin-top: 50px;
margin-top: 50px;
}
.breadcrumbs__path {
display: flex;
align-items: center;
flex: 0;
display: flex;
align-items: center;
flex: 0;
}
.breadcrumbs__path > * {
display: inline-block;
height: 30px;
padding: 5px 7px;
flex-shrink: 0;
display: inline-block;
height: 30px;
padding: 5px 7px;
flex-shrink: 0;
}
.breadcrumbs__path > *:not(.icon-breadcrumb) {
min-width: 30px;
opacity: 0.7;
min-width: 30px;
opacity: 0.7;
}
.breadcrumbs__path > *:hover {
opacity: 1;
opacity: 1;
}
.breadcrumbs__tags {
width: 300px;
flex: 1;
width: 300px;
flex: 1;
}
.breadcrumbs__tags .multiselect__tags {
border-top: none !important;
border-left: none !important;
border-right: none !important;
border-top: none !important;
border-left: none !important;
border-right: none !important;
}
.breadcrumbs__AddFolder {
margin-left: 5px;
padding: 0;
margin-top: -10px;
margin-left: 5px;
padding: 0;
margin-top: -10px;
}
.breadcrumbs__controls {
flex: 2;
display: flex;
flex-direction: row-reverse;
padding: 0;
flex: 2;
display: flex;
flex-direction: row-reverse;
padding: 0;
}
.breadcrumbs__controls > * {
min-width: 30px;
min-width: 30px;
Copy link
Member

Choose a reason for hiding this comment

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

Tabs? :p

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooops, github only displayed your comments only after I merged. Ah will fix those tabs.

@marcelklehr marcelklehr merged commit f214df8 into nextcloud:master Aug 2, 2020
@marcelklehr marcelklehr deleted the feature/unified-search branch August 2, 2020 13:30
@marcelklehr
Copy link
Member Author

But regarding bookmarks, I'm wondering if this shouldn't be done with Unified search only, no?
You only have one type of items to search in the end :)

True, but I want to support older nextcloud versions as well, so moving the search into the control bar is a good solution, yeah.

@skjnldsv
Copy link
Member

skjnldsv commented Aug 2, 2020

True, but I want to support older nextcloud versions as well, so moving the search into the control bar is a good solution, yeah.

You can still use the old search with a simple class check to see if the UnifiedSearch is enabled and active 🤷
The old standard was OCA.Search, you were already using it?
If so, just keep both and only register your OCA.Search provider if the class exists after page is done loading?

That would keep it simple and avoid you using your own input?
https://github.com/nextcloud/server/blob/b9bc2417e7a8dc81feb0abe20359bedaf864f790/core/search/js/search.js

@marcelklehr
Copy link
Member Author

Ah, nice idea.Thanks :)

@skjnldsv
Copy link
Member

skjnldsv commented Aug 2, 2020

nextcloud/server#9912

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.

2 participants