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 sharees on lookup server when explicitly requested by user #14333

Merged
merged 1 commit into from
Feb 26, 2019

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Feb 21, 2019

Before

Sharee search either never or always goes to lookup server, depending on admin setting.

Now

Sharee search by default just searches locally. If no results are returned and the lookup server is not explicitly disabled by the admin, the user gets the option to search globally. This retriggers the search request but also includes lookup server results.


Note: I had some difficulties connection to the lookup server form my dev server but the default empty response for that is sufficient for testing :)


Todo

  • Design does not look like @jancborchardt's mockup (search icon has to be moved before the text)

@jancborchardt
Copy link
Member

For the record here’s the mockup :)
search mockup

Btw @ChristophWurst, seems some unrelated files were edited maybe in the fixup?

Conflicting files

apps/accessibility/js/accessibility.js
apps/accessibility/js/accessibility.js.map

@ChristophWurst

This comment has been minimized.

@jancborchardt

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@jancborchardt

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@jancborchardt

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst
Copy link
Member Author

There seem to be a few UI hiccups here and there (already seen when working on #14180, hence this might be an old bug) but this generally works and also looks like @jancborchardt's mockup.

@schiessle

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@schiessle

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

Copy link
Member

@schiessle schiessle left a comment

Choose a reason for hiding this comment

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

Tested and works great! 👍

@ChristophWurst
Copy link
Member Author

Thanks for testing! 🙌

I squashed my fixup commits and rebased onto master.

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 25, 2019
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke

This comment has been minimized.

@rullzer

This comment has been minimized.

@MorrisJobke

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst ChristophWurst force-pushed the feature/sharee-explicit-lookup branch 2 times, most recently from 4ee34cf to 4533360 Compare February 26, 2019 08:45
@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 26, 2019
@ChristophWurst

This comment has been minimized.

@MorrisJobke

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@ChristophWurst
Copy link
Member Author

Aaaaaaah now CI looks good :) Let me do a final squash&rebase

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@rullzer rullzer merged commit 73b8b56 into master Feb 26, 2019
@rullzer rullzer deleted the feature/sharee-explicit-lookup branch February 26, 2019 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants