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

Feature/autocomplete input #228

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Feature/autocomplete input #228

wants to merge 18 commits into from

Conversation

Zarlex
Copy link

@Zarlex Zarlex commented Oct 11, 2018

Fix #228 Provide input field that has autocomplete functionality.
The list is paginated and the search is handled by the filterable. On input the filter with the value is set and then the options collection is fetched. User can select an item via keyboard navigation or by clicking it.
It has the same programmatic interface as the mwSelectBox directive

var selectedEl = el.find('#' + selected.cid),
scrollContainerEl = el.find('.auto-complete-holder');

if (selectedEl.length) {

Choose a reason for hiding this comment

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

selectedEl.length > 0, because selectedEl.length is too weak ( -1 is also true)

var setInputPadding = function () {
$timeout(function () {
if (scope.getSelectedModelLabel()) {
el.find('input').css('padding-left', el.find('.selected-item').outerWidth(true) + 15);

Choose a reason for hiding this comment

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

why not using ng-style / ng-class instead of jquery?

Copy link
Author

Choose a reason for hiding this comment

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

.find() and .css() are also methods provided by angulars jquery lite. And in this case of dynamic style calculation its easier than ng-style. ng-class would not work because of the dynamic width

Copy link

@ywachendorfer ywachendorfer Oct 15, 2018

Choose a reason for hiding this comment

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

I dont get your point with .find() and .css() are also methods... yes, they are, i know. And that fact doesn't make it better.

The question should be: is there a better approach than using heavily jQuery. And one approach could be ng-style and ng-class.

I mean from the performance view you always run through the DOM searching your elements to apply a style. The question should be: Is it necessary, or is there a way to apply the style without the search

ng-mousedown="select(model)">
{{getLabel(model)}}
</li>
<li ng-show="viewModel.isSearching" style="display: flex;justify-content: center;">

Choose a reason for hiding this comment

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

Move to a css class

</div>
<div class="auto-complete-holder"
ng-show="(mwOptionsCollection.length>0 || viewModel.isSearching) && viewModel.searchActive"
style="max-height: 115px; overflow:scroll">

Choose a reason for hiding this comment

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

Move to a css class

@@ -0,0 +1,39 @@
<div class="mw-autocomplete"
style="width: 100%">

Choose a reason for hiding this comment

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

move to a css class

* </div>
* ```
*/
.directive('mwAutocomplete', function ($q, $timeout, i18n) {

Choose a reason for hiding this comment

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

$q is not used, please remove the unnecessary injection

break;
default:
/* All letters on keyboard */
if (ev.keyCode >= 46) {

Choose a reason for hiding this comment

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

if (ev.keyCode >= 46 || ev.keyCode >= 8 || ev.keyCode >= 27) { ... }

And remove the cases:

              case 8: //backspace
              case 27: //escape
                unsetResult();
                break;

};

scope.getPlaceholder = function () {
if (scope.mwPlaceholder && !scope.getSelectedModelLabel()) {

Choose a reason for hiding this comment

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

 return scope.mwPlaceholder && !scope.getSelectedModelLabel() ? scope.mwPlaceholder : '';

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

Successfully merging this pull request may close these issues.

2 participants