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

Search bar in top bar #3460

Merged
merged 1 commit into from
Jun 3, 2020
Merged

Search bar in top bar #3460

merged 1 commit into from
Jun 3, 2020

Conversation

LukasHirt
Copy link
Contributor

@LukasHirt LukasHirt commented May 13, 2020

Description

Moved search bar from app bar into top bar

Screenshots (if appropriate):

host docker internal_8300_index html(iPad Pro)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

@LukasHirt
Copy link
Contributor Author

For now, it is built on top of the sidebar PR. After it'll get merged, I'll drop that commit in here.

@LukasHirt
Copy link
Contributor Author

LukasHirt commented May 14, 2020

There is one issue with search on mobile. Since we hide it in the drop, this results in the search being still visible after the search has been finished. However, if we use the closeOnClick then it works if we execute the search by clicking on the search button but it also hides it if we click only into the input. And of course, if the search is executed via keyboard then the result is the same as in the beginning.

It's times like these I really wish for having drop with a simple v-if instead of the UIkit one...

@LukasHirt LukasHirt force-pushed the feature/search-in-topbar branch 3 times, most recently from ceec420 to 74212da Compare May 27, 2020 08:06
@LukasHirt LukasHirt marked this pull request as ready for review May 27, 2020 08:06
@LukasHirt LukasHirt self-assigned this May 27, 2020
@LukasHirt
Copy link
Contributor Author

Fixed the selector in tests for loading spinner in the search bar. Tests are now passing locally. Also, I've hidden the top bar in public links on wider screens because they had no content there

@LukasHirt
Copy link
Contributor Author

Ok, one more change - hidden top bar and sidebar in public pages. Top bar because there is no element which could be used (opened a ticket to make search work in public links #3521) and sidebar because there are no nav items (opened a ticket for branding in such case #3520)

@LukasHirt
Copy link
Contributor Author

Tests are green. Pls review @PVince81 @kulmann

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Only two small comments. Looks cool in the topbar! I have two things about the searchbar itself, but will raise that feedback in ODS where it belongs.

src/components/SearchBar.vue Outdated Show resolved Hide resolved
src/components/SearchBar.vue Show resolved Hide resolved
@LukasHirt
Copy link
Contributor Author

LukasHirt commented Jun 2, 2020

@kulmann Label translated with message "Open search bar" and search reset when hitting enter added.

@LukasHirt LukasHirt merged commit 41e1507 into master Jun 3, 2020
@LukasHirt LukasHirt deleted the feature/search-in-topbar branch June 3, 2020 07:46
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