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

Live search view #176

Closed
bloodbare opened this issue Feb 11, 2014 · 23 comments
Closed

Live search view #176

bloodbare opened this issue Feb 11, 2014 · 23 comments

Comments

@bloodbare
Copy link
Member

No description provided.

@davisagli
Copy link
Member

mockup already has a livesearch pattern

@bloodbare
Copy link
Member Author

We need to apply it to plone5 and skin it.

@djay
Copy link
Member

djay commented Feb 11, 2014

Can we just remove it completely from the public theme? We have it already
in toolbar structure. For public use it is a scalability nightmare. This
also makes the site easier to theme.

@vangheem
Copy link
Member

I know it might be a minority position but I also agree with @djay on this.

@davisagli
Copy link
Member

There is already a setting for it so it's easy to toggle off if you have scalability or theming problems with it.

@djay
Copy link
Member

djay commented Feb 11, 2014

If someone turns that feature back on and the theme doesn't support it? And
all the mockup code that has to go into the frontend theme to support it?
It seems much more like a plugin or 3rd party theme feature to me.
On 12 Feb 2014 09:34, "David Glick" [email protected] wrote:

There is already a setting for it so it's easy to toggle off if you have
scalability or theming problems with it.

Reply to this email directly or view it on GitHubhttps://github.com//issues/176#issuecomment-34816859
.

@davisagli davisagli mentioned this issue Feb 25, 2014
90 tasks
@tisto
Copy link
Member

tisto commented Jul 25, 2014

I want to keep that feature in Plone and I think we should enable it by default. If people evaluate Plone, it is important that we provide such a common feature. It is way easier for experienced users to turn that off than for newbies to turn it on.

@thet
Copy link
Member

thet commented Jul 25, 2014

absolutly.

that feature was what impressed me most when i first encountered plone,
back in 2.something days ;)

but we should refactor it. the markup is way to complicated. but that's
what the livesearch pattern is about?

On Thu, 2014-07-24 at 22:40 -0700, Timo Stollenwerk wrote:

I want to keep that feature in Plone and I think we should enable it
by default. If people evaluate Plone, it is important that we provide
such a common feature. It is way easier for experienced users to turn
that off than for newbies to turn it on.


Reply to this email directly or view it on GitHub.

programmatic web development
di(fh) johannes raggam / thet
python plone zope development
plone framework team member
mail: [email protected]
web: http://programmatic.pro
http://bluedynamics.com

@djay
Copy link
Member

djay commented Jul 25, 2014

I think it should be considered carefully.
If it's enabled by default we are leaving in a feature that makes the site harder to scale. If a person doesn't understand scalability well they might blame it on plone rather realising it is this feature.
Also my reasoning for saying live search makes sites harder to theme is this. Every out of the box theme will have to support it since it is the default and due to the overlay it can be very fiddly. We are putting another barrier in the way of creating a culture of plone being easy to theme and having many freely available themes. I think it is very important for the future of plone that it be as easy as possible to theme (as well as host and use). If we can attract designers the community will grow.
If you are making you own custom theme for a client and you decide to turn off the feature to not deal with it... it is just a switch for the client to turn on. If they turn it on, they will complain to you that your theme is broken and blame you.
My suggestion is that it should be a plugin included in the core but not turned on by default, like CMFEditions. For demos it can be installed. If a first time user installs plone they will see it in the activation list and likely install it to play with it, so I think new users will still be impressed with it. But it's description could come with a warning that it can result in a lot of extra requests which can have a performance impact on your site.

@thet
Copy link
Member

thet commented Jul 25, 2014

the theming part can be solved by a refactoring to a simpler markup. IMO the livesearch overlay markup is too complicated and not consistent (e.g. no results is different from results).

regarding scalability: can you please describe a scenario, where it leads to scalability troubles? IMO, it's just a search field, people will use anyways if they want to find something. maybe raising the threshold from where ajax requests are submitted will solve the problem?

@acsr
Copy link

acsr commented Jul 25, 2014

For me setting up the treshhold when the search is triggered to 4 letters years ago, helped a lot. But there was never a UI to configure that.

I agree with the complains by djay, but fixing issues by just cutting features is not the first solution.

Rethinking how a customer demand can be managed in a new way ist the good part of it. Lets do that.
The only issue when cutting off the feature is from my point to first dprecate it and then remove it. Otherwise it will be unfair. Even for a free software.

@tisto
Copy link
Member

tisto commented Jul 25, 2014

Implementing the live search on top of Bootstrap Typeahead or Select2 should be trivial and we should of course use sensible defaults.

Removing livesearch in Plone 5 would require a PLIP. @djay Feel free to submit a PLIP, then we can discuss this in the FWT. My personal opinion is still that we should keep it for the reasons expressed above.

@djay
Copy link
Member

djay commented Jul 25, 2014

How many custom themes for public sites have you guys done and left live
search turned on?

BTW my point wasn't that its incredibly complex to theme live search but
that it is just more things to theme and get designed. More things to theme
the greater the cost. It all adds up. Site map, contact page from 404,
various table and list styles, these are all compulsory elements of a plone
theme that add extra to the cost and difficulty of a plone theme and aren't
always used.
Also not sure about requiring bootstrap for theming.

@vangheem
Copy link
Member

It'd be easy enough to modernize it, make it opt-in with the theme(applying classes or something). We already ship with some bootstrap patterns(js). It doesn't require to be bootstrap in order to use the patterns though.

Let's implement it, make it better, perhaps easier to scale, etc.

@fulv
Copy link
Member

fulv commented Jul 26, 2014

One effort that really pays off in making the theming work easier is to
update the test_rendering page so that it covers as many styling
requirements as possible. Granted, for an overlay feature like livesearch,
I'm not sure how feasible it is to have it rendered on a static page, but
it shouldn't be too hard.

Fulvio

On Fri, Jul 25, 2014 at 10:34 AM, Nathan Van Gheem <[email protected]

wrote:

It'd be easy enough to modernize it, make it opt-in with the
theme(applying classes or something). We already ship with some bootstrap
patterns(js). It doesn't require to be bootstrap in order to use the
patterns though.

Let's implement it, make it better, perhaps easier to scale, etc.

Reply to this email directly or view it on GitHub
#176 (comment)
.

@ebrehault
Copy link
Member

In order to release Plone 5 beta, I think we should just integrate the Mockup live search pattern (the discussion about removing live search should be done via a PLIP).

Any taker?

@thet
Copy link
Member

thet commented Nov 28, 2014

@ale-rt was working on it during our ploneconf mockup training. here is his branch: https://github.com/plone/mockup/compare/282-livesearch.js adn this is an issue handling it #287
I'm not sure about the status - @ale-rt ?

When talking with Alessandro, he pointed out a very nice idea: the livesearch should use @@search as it's backend. @@search should return a JSON response. This way, we'd get an awesome search API.

Another thing: we should make the timeout between keypresses and the minimum characters configurable and also let admins easily disable livesearch. The biggest complaint against the livesearch view is, that it can bring down the site by too much requests from clients. These changes would help tuning it.

@vangheem
Copy link
Member

no need to use a special json view response. Just use the existing query api that's already available. We already have a robust, query json api.

@ale-rt
Copy link
Member

ale-rt commented Nov 28, 2014

The status of my work is summarized here:

With @thet were wondering if it's possible to have a json or html response from the @@search depending on the headers: if they contain traces of ajax we could return json.

The search delay should be configurable setting this parameter:

@thet
Copy link
Member

thet commented Nov 28, 2014

@vangheem do we? give a pointer, please...

@vangheem
Copy link
Member

vangheem commented Feb 8, 2015

bah, I just implement this and I didn't know that @ale-rt did all the of the work already... Sigh.

@ale-rt would you might looking at my implementation and seeing if you have any additional improvements? plone/mockup@3004bca

This is now integrated into plone core live search.

@vangheem vangheem closed this as completed Feb 8, 2015
@ale-rt
Copy link
Member

ale-rt commented Feb 8, 2015

Well done @vangheem!
I really prefer your version, mine was basically moving the old Plone code to a pattern.
Your approach is really preferable :)

Thanks a lot for closing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests