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

Overlay-based components should handle page scroll gracefully #4093

Closed
jelbourn opened this issue Apr 13, 2017 · 9 comments · Fixed by #5134
Closed

Overlay-based components should handle page scroll gracefully #4093

jelbourn opened this issue Apr 13, 2017 · 9 comments · Fixed by #5134
Assignees
Labels
P2 The issue is important to a large percentage of users, with a workaround

Comments

@jelbourn
Copy link
Member

Creating this issue to track this problem for menu, tooltip, select, autocomplete, datepicker, dialog, snackbar.

Approach we will be taking:

  • Default behavior will be to reposition the overlay on scroll using the ScrollDispatcher
  • Behavior can be changed to disable scrolling instead on a strictly opt-in basis
  • Implementation of scroll-blocking must be overridable by end-user
@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Apr 13, 2017
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 26, 2017
* Adds the `scrollStrategy` option to the overlay state, allowing the consumer to specify what scroll handling strategy they'd want to use. Also includes a `ScrollStrategy` interface that users can utilize to build their own strategies.
* Adds the `RepositionScrollStrategy`, `CloseScrollStrategy` and `NoopScrollStrategy` as initial, out-of-the-box strategies.
* Sets the `RepositionScrollStrategy` by default on all the connected overlays and removes some repetitive logic from the tooltip, autocomplete, menu and select.

**Note:** I'll add a `BlockScrollStrategy` in a follow-up PR. I wanted to keep this one shorter.

Relates to angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Apr 29, 2017
* Adds the `scrollStrategy` option to the overlay state, allowing the consumer to specify what scroll handling strategy they'd want to use. Also includes a `ScrollStrategy` interface that users can utilize to build their own strategies.
* Adds the `RepositionScrollStrategy`, `CloseScrollStrategy` and `NoopScrollStrategy` as initial, out-of-the-box strategies.
* Sets the `RepositionScrollStrategy` by default on all the connected overlays and removes some repetitive logic from the tooltip, autocomplete, menu and select.

**Note:** I'll add a `BlockScrollStrategy` in a follow-up PR. I wanted to keep this one shorter.

Relates to angular#4093.
andrewseguin pushed a commit that referenced this issue May 2, 2017
* feat(overlay): add scroll handling strategies

* Adds the `scrollStrategy` option to the overlay state, allowing the consumer to specify what scroll handling strategy they'd want to use. Also includes a `ScrollStrategy` interface that users can utilize to build their own strategies.
* Adds the `RepositionScrollStrategy`, `CloseScrollStrategy` and `NoopScrollStrategy` as initial, out-of-the-box strategies.
* Sets the `RepositionScrollStrategy` by default on all the connected overlays and removes some repetitive logic from the tooltip, autocomplete, menu and select.

**Note:** I'll add a `BlockScrollStrategy` in a follow-up PR. I wanted to keep this one shorter.

Relates to #4093.

* fix: missing types on the scroll dispatcher

* refactor: use class for fake scroll strategy

* refactor: add onAttached and onDetached observables

* chore: rename observables
@sabithpocker
Copy link

Auto complete overlayed dropdown position is not updated on scroll.

@sabithpocker
Copy link

sabithpocker commented May 12, 2017

Autocomplete does support repositioning upon resize / scroll. But not when dropdown list items change and there by height of dropdown.
Also why is the position dependent on height of dropdown? it can be accommodated above/below just based on availability of space? Or is there a priority to show it below?

Consider a case where one option is already entered("png-alpha" in screenshot). Now when focus is back in the input there is just enough space below the control to show one option(already selected one) and it fits in.

screen shot 2017-05-12 at 3 22 20 pm

Figure 1: single item accommodated in just-enough-space

But if we delete couple of characters from the input text, more options showup but doesn't update position to above neither does it allow to scroll beyond document height.

screen shot 2017-05-12 at 3 26 28 pm

Figure 2: broadening search critereon brings in more items but not accessible via scroll/neither does it reposition

If I do a scroll or blur/focus-back the repositioning is triggered correctly, but not upon change in "input text" or "dropdown-list" length.
screen shot 2017-05-12 at 3 30 31 pm
_Figure 3: clicking out to blur and focusing in back fixes the issue and triggers repositioning _

This can be fixed by either of the following in my opinion:
1. Updating position when input text changes along with scroll, which might eat up performance.
2. Make position(above/below) irrespective of height of dropdown and only based on proximity of input element to screen edges. This way figure 1 should show the options above irrespective of the fact that it can be accommodated below.

using master "@angular/material": "git+https://github.com/angular/material2-builds.git"

@crisbeto
Copy link
Member

#4469 will reposition the autocomplete panel when the amount of items changes.

crisbeto added a commit to crisbeto/material2 that referenced this issue May 12, 2017
Adds the `BlockScrollStrategy` which, when activated, will prevent the user from scrolling. For now it is only used in the dialog.

Relates to angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 12, 2017
Adds the `BlockScrollStrategy` which, when activated, will prevent the user from scrolling. For now it is only used in the dialog.

Relates to angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 12, 2017
Adds the `BlockScrollStrategy` which, when activated, will prevent the user from scrolling. For now it is only used in the dialog.

Relates to angular#4093.
jelbourn pushed a commit that referenced this issue May 15, 2017
Adds the `BlockScrollStrategy` which, when activated, will prevent the user from scrolling. For now it is only used in the dialog.

Relates to #4093.
@sudheeshcm
Copy link

sudheeshcm commented May 24, 2017

@crisbeto @jelbourn
Isn't this fix merged in the latest beta version(2.0.0-beta.5). It works in the example provided.
Still my autocomplete options scrolls with the page.

If not please let me know, how to include those changes.

Thanks,
Sudheesh

@Zefling
Copy link

Zefling commented May 24, 2017

@sudheeshcm Are you in this case : #4557 ?

@sudheeshcm
Copy link

@Zefling I'm not exactly sure. Tried removing all the "overflow: auto" styles, and still have this issue.

@tcrack326
Copy link

Datepicker seems to be in a fixed position and does not stay in same position when scrolling away from input. This may be intentional to keep the user from scrolling away before making a choice but if you have a datepicker near the bottom of a page then part of the datepicker will be hidden.

@crisbeto
Copy link
Member

There's a pending PR (#4696) that adds positioning improvements to the datepicker.

crisbeto added a commit to crisbeto/material2 that referenced this issue May 28, 2017
…e/override custom strategies

* Refactors the overlay setup to allow for scroll strategies to be passed in by name, instead of by instance.
* Handles the scroll strategy dependency injection automatically.
* Adds an API for registering custom scroll strategies and overriding the existing ones.
* Adds a second parameter to the `attach` method, allowing for a config object to be passed in.
* Throws an error if there's an attempt to attach a scroll strategy multiple times. This is mostly a sanity check to ensure that we don't cache the scroll strategy instances.

Relates to angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 28, 2017
…e/override custom strategies

* Refactors the overlay setup to allow for scroll strategies to be passed in by name, instead of by instance.
* Handles the scroll strategy dependency injection automatically.
* Adds an API for registering custom scroll strategies and overriding the existing ones.
* Adds a second parameter to the `attach` method, allowing for a config object to be passed in.
* Throws an error if there's an attempt to attach a scroll strategy multiple times. This is mostly a sanity check to ensure that we don't cache the scroll strategy instances.

Relates to angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 2, 2017
…e/override custom strategies

* Refactors the overlay setup to allow for scroll strategies to be passed in by name, instead of by instance.
* Handles the scroll strategy dependency injection automatically.
* Adds an API for registering custom scroll strategies and overriding the existing ones.
* Adds a second parameter to the `attach` method, allowing for a config object to be passed in.
* Throws an error if there's an attempt to attach a scroll strategy multiple times. This is mostly a sanity check to ensure that we don't cache the scroll strategy instances.

Relates to angular#4093.
andrewseguin pushed a commit that referenced this issue Jun 5, 2017
…e/override custom strategies (#4855)

* feat(overlay): more flexible scroll strategy API and ability to define/override custom strategies

* Refactors the overlay setup to allow for scroll strategies to be passed in by name, instead of by instance.
* Handles the scroll strategy dependency injection automatically.
* Adds an API for registering custom scroll strategies and overriding the existing ones.
* Adds a second parameter to the `attach` method, allowing for a config object to be passed in.
* Throws an error if there's an attempt to attach a scroll strategy multiple times. This is mostly a sanity check to ensure that we don't cache the scroll strategy instances.

Relates to #4093.

* refactor: switch to new approach without ReflectiveInjector

* refactor: switch to better approach

* refactor: switch to even simpler approach

* chore: address feedback
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 14, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 15, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 17, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 22, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 23, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
mmalerba pushed a commit that referenced this issue Jul 6, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes #4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 8, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 14, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 17, 2017
… component

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes angular#4093.
jelbourn pushed a commit that referenced this issue Jul 17, 2017
… component (#5134)

Adds providers to the autocomplete, connected overlay, datepicker, dialog, menu, select and tooltip components, that allow for the default scroll strategy to be overwritten.

Fixes #4093.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants