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

Fix EZP-21239: Autocomplete using YUI3 #122

Merged
merged 2 commits into from Oct 2, 2013

Conversation

yannickroger
Copy link
Contributor

Description

Link to the issue: https://jira.ez.no/browse/EZP-21239

The autocomplete feature of ezfind doesn't work when using 2 of the 3 japanese alphabets. These alphabets are not supported buy YUI2 autocomplete, so I rewrote the feature using YUI3 autocomplete which supports them.

Screenshots

Admin

autocomplete_admin_top

autocomplete_admin_page

eZdemo

autocomplete_ezdemo_page

eZflow

autocomplete_ezflow_top

autocomplete_ezflow_page

Test

Manual test using FF, chrome and IE

};
loader.insert( {}, 'js' );
}
YUI().use('autocomplete', 'autocomplete-highlighters', 'datasource-io','json-parse', function (Y) {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of explicitly requiring some modules here, I would set them as dependencies of the ezfindautocomplete one. (see second example in http://yuilibrary.com/yui/docs/yui/#yuiadd)
(nitpick: there's also a small CS issue, missing space before 'json-parse')

@dpobel
Copy link
Contributor

dpobel commented Sep 30, 2013

quite ok for a first frontend related PR ;-)
Can you add a screenshot of the autocomplete in ezdemo and in the admin, just to see what it looks like now?

@yannickroger
Copy link
Contributor Author

@dpobel 's suggestions have been taking care of. The PR also includes another commit to rename admin2 since it was supposed to be the case since 5.0.

};
loader.insert( {}, 'js' );
}
YUI().use('ezfindautocomplete', function (Y) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you're already in the ezfindautocommplete module, so you don't need to require it (I'm actually surprised this is working :))

@dpobel
Copy link
Contributor

dpobel commented Oct 1, 2013

besides the inline comment, +1

@yannickroger
Copy link
Contributor Author

Fixed @dpobel 's last comment. Anybody else who loves javascript ? @andrerom ?

@lserwatka
Copy link
Member

I have only two questions. Did you test a case when there are 2 or more search blocks on one page with eZ Flow? Or in admin when autocomplete is enabled for header and in the search result page?

@yannickroger
Copy link
Contributor Author

@lserwatka : yes and yes
On the first ezflow screenshot, you can see that they are both on the same page.

@andrerom
Copy link
Contributor

andrerom commented Oct 2, 2013

Big plus one on on getting rid of some yui2 and jquery use 👍

@yannickroger yannickroger merged commit f3c4c01 into master Oct 2, 2013
@yannickroger yannickroger deleted the ezp-21239_upgrade_to_YUI3_autocomplete_for_jap branch October 2, 2013 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants