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

[WIP]: Fulltext search (#8381) #8714

Closed

Conversation

mrsimpson
Copy link
Collaborator

@geekgonecrazy this PR is only used to discuss WIP of search interface and sample implementation
Addresses RocketChat/feature-requests#745

rodrigok and others added 30 commits August 24, 2017 22:00
…ignment

[FIX] message actions over unread bar
* Fix hyperlink style on sidebar footer

* fix a selector
…ow-rocketDebug

[FIX] Window exception when parsing Markdown on server
…e-api-notifications

[FIX] Remove break change in Realtime API
[FIX] Fix google play logo on repo README
…irst-load

[FIX] Show leader on first load
[DOCS] Add native mobile app links into README and update button images
…placeholders

[FIX] Fix placeholders in account profile
[FIX] Document README.md. Drupal repo out of date
[FIX] status and active room colors on sidebar
…list

[FIX] Fix the status on the members list
…de-render

[FIX] Markdown being rendered in code tags
@mrsimpson
Copy link
Collaborator Author

I’ll do a rebase soon, that will make it easier. Was in a hurry yday. It’s actually currently only two packages.

@mrsimpson
Copy link
Collaborator Author

@geekgonecrazy @graywolf336 merged develop. Didn't notice the branch-off-point before I pushed it. Sorry for that!
I would really try to stick to those two packages. I plan to integrate via webhooks which then in their implementation trigger the active search provider.
Thus, I'd only add hooks if needed.
With respect to the UI I am uncertain. I don't like the integration in the tabbar, as this one currently serves only purposes in the context of the room - and search is cross-room.
Therefore, I'd go and integrate into spotlight. We'll check which UI suits us. Ok?

@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@RocketChat RocketChat deleted a comment Nov 1, 2017
@geekgonecrazy
Copy link
Contributor

With respect to the UI I am uncertain. I don't like the integration in the tabbar, as this one currently serves only purposes in the context of the room - and search is cross-room.
Therefore, I'd go and integrate into spotlight. We'll check which UI suits us. Ok?

Regarding UI I know the sidebar is changing. I think search will move to the top and it may become a bit more fitting for global.

But at any rate I think for the scope of implementing search providers and search provider api's we should provide methods to be called to perform the search, but we should not go so far as to hook into the UI.

This way the frontend guys can work with designers to get that all figured out.

*/
class RedlinkSearchProviderUi extends SearchProviderUi {

}
Copy link
Contributor

Choose a reason for hiding this comment

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

If implemented what would this do beyond the settings exposed?

Copy link
Collaborator Author

@mrsimpson mrsimpson Nov 3, 2017

Choose a reason for hiding this comment

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

@geekgonecrazy there are usecases where a search provider has own capabilities, such as filtering, facetting, more-like-this, ...
A generic UI would have to be highly configurable in order to deal with all the different options - and it would be tricky to get all providers aligned.
In addition to that, not all provides might be able to integrate with blaze. In this case, they could provide an empty template with a unique selector and provide the UI from the external system (loaded lazily). This makes the provider independent from RC-releases as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@graywolf336 thoughts? 🔝 this is going to be important across various parts of rocketchat apps.

Also we don't want to count on blaze.... as we might be switching frameworks. So something agnostic would be best if this is needed.

Copy link
Collaborator Author

@mrsimpson mrsimpson Nov 6, 2017

Choose a reason for hiding this comment

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

@geekgonecrazy one more thing about that: many external systems (which are to be integrated) already bring some sort of UI. For search, there are plenty of libs. None of them is written in blaze. Thus, a template which loads the other technology‘s UI onCreated is at least something to consider. As you said: also for the RC apps. See https://github.com/assistify/Rocket.Chat/blob/develop/packages/assistify-ai/client/views/app/tabbar/smarti.js

@geekgonecrazy
Copy link
Contributor

@mrsimpson do we want to close this in favor of #10110 ?

@mrsimpson
Copy link
Collaborator Author

Of course! This one is kind-of the forerunner ;) Looking so much forward to seeing #10110 merged!

@mrsimpson mrsimpson closed this Apr 11, 2018
@mrsimpson mrsimpson deleted the core/#8381-fulltext-search branch December 7, 2020 09:08
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.

9 participants