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 ability for route meta options to hide search bar #4815

Merged
merged 3 commits into from
Apr 7, 2021

Conversation

rpocklin
Copy link
Contributor

@rpocklin rpocklin commented Mar 17, 2021

Description

Simply the ability to toggle on and off the search bar in the files app per-route, so you can customise the interface further depending on what you are showing as content (ie. a file preview or other viewer).

Motivation and Context

See description.

How Has This Been Tested?

Tested working locally, it's very simple change.

Screenshots (if appropriate):

With
files-with-search-bar
Without
files-no-search-bar

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

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs

This comment has been minimized.

@rpocklin rpocklin force-pushed the feat/enable-togglable-search-bar branch 3 times, most recently from 824da0f to 9d367e4 Compare March 18, 2021 01:30
@kulmann
Copy link
Contributor

kulmann commented Apr 2, 2021

@rpocklin thank you for this PR, especially for including tests! There have been changes to master in the meantime, could you rebase this PR and update the test snapshot again? Happy to merge it then. :-)

@rpocklin rpocklin force-pushed the feat/enable-togglable-search-bar branch from 9d367e4 to bc41d21 Compare April 7, 2021 01:24
@rpocklin
Copy link
Contributor Author

rpocklin commented Apr 7, 2021

@rpocklin thank you for this PR, especially for including tests! There have been changes to master in the meantime, could you rebase this PR and update the test snapshot again? Happy to merge it then. :-)

@kulmann done :) Really like the way unit testing has been implemented btw.

@kulmann
Copy link
Contributor

kulmann commented Apr 7, 2021

Thanks! 💪 I restarted CI, because it seems like the failure was random (there are a few unstable scenarios right now, sorry for that). Will hit merge as soon as CI is green.

@kulmann kulmann merged commit fa1fa8d into owncloud:master Apr 7, 2021
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