Add md-dropdown-position option to mdAutocomplete. #9774

donmccurdy · 2016-10-06T21:19:29Z

Because obstructions in the viewport (liked fixed-position elements) cannot be automatically detected, allow users to override top/bottom positioning when needed.

Usage:

<md-autocomplete md-items="item in ctrl.querySearch(ctrl.searchText)"
                 md-dropdown-position="bottom"
                 ...

Fixes #9769.

devversion

This looks good.

Having unit tests for this attribute would be good (Just checking for the top or bottom attribute, to detect the alignment)

devversion · 2016-10-06T21:37:10Z

src/components/autocomplete/js/autocompleteController.js

@@ -106,7 +106,12 @@ function MdAutocompleteCtrl ($scope, $element, $mdUtil, $mdConstant, $mdTheming,
        left   = hrect.left - root.left,
        width  = hrect.width,
        offset = getVerticalOffset(),
+        position = $attrs.mdDropdownPosition,


Can you use $scope.dropdownPosition to support values which can change during runtime?

Oh, so should this be a text binding or two-way?

I think text binding would make more sense here (mdDropdownPosition: '@?')

devversion · 2016-10-06T21:39:25Z

src/components/autocomplete/js/autocompleteController.js

        styles;
+    // Automatically determine dropdown placement based on available space in viewport.


While you are at it, can you add an empty line before the comment (just looks better then)

donmccurdy · 2016-10-07T15:45:29Z

I am having trouble running the tests — is there documentation somewhere on that? I couldn't find anything in https://github.com/angular/material/blob/master/.github/CONTRIBUTING.md.

When running gulp karma I get:

Cannot load browser "Chrome": it is not registered!

I have at least two versions of Chrome installed. ;)

devversion · 2016-10-07T15:47:18Z

I'm sorry, yeah we have not included the chrome karma launcher right now (accidentally) but it will be fixed soon. Try doing npm i karma-chrome-launcher for now, to get everything working.

donmccurdy · 2016-10-07T18:00:37Z

Thanks — I've added a test case now.

devversion

Thanks. Please fix those two minor comments, then this should be ready.

devversion · 2016-10-07T18:25:40Z

src/components/autocomplete/autocomplete.spec.js

+          'md-items="item in match(searchText)" ' +
+          'md-item-text="item" ' +
+          'md-min-length="0" ' +
+          'md-dropdown-position="' + dropdownPosition +'"' +


Can you just create a scope variable for this, so you can just update the variable later?

<x md-dropdown-position="{{ position }}">

// Then later you could just do the following scope.$apply('position = "bottom"');

devversion · 2016-10-07T18:26:07Z

src/components/autocomplete/autocomplete.spec.js

+      expect(scrollContainer.style.top).toMatch(/[0-9]+px/);
+      expect(scrollContainer.style.bottom).toBe('auto');
+
+      document.body.removeChild(parent[0]);


Can be simplified to

parent.remove();

devversion · 2016-10-18T18:41:52Z

@donmccurdy Just looked again on this and it LGTM. Can you please rebase the PR if possible?

ThomasBurleson · 2016-10-18T19:53:18Z

@donmccurdy - after rebase, lgtm.

Because obstructions in the viewport (liked fixed-position elements) cannot be automatically detected, allow users to override top/bottom positioning when needed. Usage: ```html <md-autocomplete md-items="item in ctrl.querySearch(ctrl.searchText)" md-dropdown-position="bottom" ... ``` Fixes #9769.

donmccurdy · 2016-10-18T21:15:24Z

@devversion @ThomasBurleson Thanks! I've rebased against master. The Travis CI build stalled, though, is that likely to be related to my code?

devversion self-assigned this Oct 6, 2016

devversion added the needs: review This PR is waiting on review from the team label Oct 6, 2016

devversion reviewed Oct 6, 2016

View reviewed changes

devversion added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Oct 6, 2016

devversion mentioned this pull request Oct 7, 2016

chore: add karma chrome launcher #9778

Merged

devversion approved these changes Oct 7, 2016

View reviewed changes

devversion removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Oct 7, 2016

devversion assigned ThomasBurleson and unassigned devversion Oct 7, 2016

naomiblack added the g3: pr This PR was posted by an internal or external product team. label Oct 18, 2016

devversion added needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved and removed needs: review This PR is waiting on review from the team labels Oct 18, 2016

ThomasBurleson removed the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Oct 18, 2016

ThomasBurleson added the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Oct 18, 2016

ThomasBurleson added needs: presubmit and removed needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved labels Oct 18, 2016

ThomasBurleson added this to the 1.1.2 milestone Oct 18, 2016

jelbourn merged commit 1ed298b into angular:master Oct 21, 2016

razorhenry1 mentioned this pull request Nov 19, 2017

Add dropdown-position option to autocomplete. angular/components#8540

Closed

		styles;
		// Automatically determine dropdown placement based on available space in viewport.

Add md-dropdown-position option to mdAutocomplete. #9774

Add md-dropdown-position option to mdAutocomplete. #9774

Uh oh!

Conversation

donmccurdy commented Oct 6, 2016

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donmccurdy commented Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devversion commented Oct 7, 2016

Uh oh!

donmccurdy commented Oct 7, 2016

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion commented Oct 18, 2016

Uh oh!

ThomasBurleson commented Oct 18, 2016

Uh oh!

donmccurdy commented Oct 18, 2016

Uh oh!

Uh oh!

donmccurdy commented Oct 7, 2016 •

edited

Loading