ezsystems / eztags

GNU General Public License v2.0
40 stars 40 forks source link

eZ Tags auto-complete: match before or after the input phrase #106

Closed thiagocamposviana closed 8 years ago

thiagocamposviana commented 8 years ago

This pull request makes it possible for the user to control when the auto complete field should fetch tags matching the beginning of the string or if the it should contain the string in the middle of the tag name.

This is specially useful when we have thousands of tags and we don't want to create more, so we need also to fetch and verify if there are tags with the current string in the middle of its name.

For example, right now if we search for "test", it will return tags like "test 1" and "test 2", but not "this is a test", with this commit it makes it possible to control the search so it also returns "this is a test".

The commit creates a checkbox, "Broad Match", in the attribute edit view, if checked the behavior changes to match the specified case.

@peterkeung

emodric commented 8 years ago

Hi!

I'm not sure an additional option is really needed. Why not just search in the middle of the tag keyword by default? It's not a BC break and it really makes things simpler.

thiagocamposviana commented 8 years ago

That is because it makes it easier for the user to decide when to search in one way or another.

peterkeung commented 8 years ago

When you have thousands of tags it does make a difference -- sometimes you want to see results for "starts with" only but other times not.

emodric commented 8 years ago

The thing is, I don't like a checkbox in the interface as it adds another level of complexity.

What of custom interfaces that don't have a search field? This config is global, but only applied to one type of interface.

peterkeung commented 8 years ago

We could move the BroadMatchAutoComplete setting to the [EditSettings] block. I think it's descriptive since it mentions "AutoComplete". Instead of a checkbox, we could support some wildcard entered by the user in the field, but we would need some instructions around that.

emodric commented 8 years ago

Ini would be okay, as long as it's not in the interface, no need for wildcards.

A setting that accepts only start, only end, or everywhere would be good. EditSettings/AutocompleteType for example, with default set to start.

peterkeung commented 8 years ago

We're going to ask our client who is using this feature how useful / not useful the current interface toggle is to them.

emodric commented 8 years ago

Hi @peterkeung @thiagocamposviana

Any news on this?

peterkeung commented 8 years ago

Sorry, the client had a personal issue so we are still waiting for a response.

peterkeung commented 8 years ago

Response: they don't toggle the option. They've just kept it on "broad match" the entire time.

emodric commented 8 years ago

So INI file is a good solution?

peterkeung commented 8 years ago

INI good for now. Remembering that we have a small sample size, some option could be useful in the future. But we could build on this first version as needed.

thiagocamposviana commented 8 years ago

Hi

I will take into this later tomorrow probably.

Thiago Campos Viana

eZ Publish Certified Developer: http://auth.ez.no/certification/verify/376924

On Thu, Jun 30, 2016 at 1:31 PM, Peter Keung notifications@github.com wrote:

INI good for now. Remembering that we have a small sample size, some option could be useful in the future. But we could build on this first version as needed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ezsystems/eztags/pull/106#issuecomment-229713796, or mute the thread https://github.com/notifications/unsubscribe/AAVjWHa1Avvd7otOel1WU_fNUropA6kSks5qQ-91gaJpZM4Iqytw .

thiagocamposviana commented 8 years ago

I have updated and now it simply uses the ini setting.

thiagocamposviana commented 8 years ago

Hi

Any update on this one? I am planning to create a new pull request soon but I want to first wait for this one.

emodric commented 8 years ago

Hey @thiagocamposviana!

I've left a few comments for you, if you can address them today, I might merge, otherwise, it will need to wait August as I'm on vacation.

thiagocamposviana commented 8 years ago

I have fixed the code logic and also the typo, but I still kept the BroadMatchAutoComplete because I still think it is kind of clear.

emodric commented 8 years ago

Hi! I still don't like "broad match" nomenclature. Can you please rename to AutoCompleteType with options "start/any" and I'll merge.

emodric commented 8 years ago

@thiagocamposviana I'm planning to release a new version soon with all the patches from the past couple of months and I'd like to include this as well. Do you want to rename the option or should I?

peterkeung commented 8 years ago

@emodric I'm sure he will want to, but he is on vacation until next week :)

emodric commented 8 years ago

@peterkeung I'm away for the whole next week due to web summer camp, so it will have to wait for me to return then :)

I can do it now if you agree.

peterkeung commented 8 years ago

Sure, go for it!

emodric commented 8 years ago

@peterkeung Looks good? dec5c21267dcf8ed7691c506838a86e2e5904fda

Now we can expand the config to add other match types without worrying about breaking something.

peterkeung commented 8 years ago

Great!

emodric commented 8 years ago

Closing as it is merged manually.

emodric commented 8 years ago

@peterkeung P.S. I just released 2.2. Thanks for all the patches!