Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-virtual-repeat scroll size not updated #4435

Closed
bluebirdtech opened this issue Sep 2, 2015 · 19 comments
Closed

md-virtual-repeat scroll size not updated #4435

bluebirdtech opened this issue Sep 2, 2015 · 19 comments
Assignees
Milestone

Comments

@bluebirdtech
Copy link

If the number of items shrinks, and scrollOffset > 0, it scrolls to the top, but doesn't update the scroll size. Hence, you're left with a dirty scrollbar.

This is happening here: https://github.com/angular/material/blob/master/src/components/virtualRepeat/virtual-repeater.js#L543

Not familiar with this code, but perhaps it needs to call this.container.setScrollSize(itemsLength * this.itemSize);

In addition to this.container.resetScroll();

Easy fix?

@ThomasBurleson ThomasBurleson assigned kasperl and kseamon and unassigned kasperl Sep 2, 2015
@ThomasBurleson ThomasBurleson added this to the 0.12.0 milestone Sep 2, 2015
@ngraef
Copy link
Contributor

ngraef commented Sep 2, 2015

Possibly related to #4308?

@MCSCC
Copy link

MCSCC commented Sep 9, 2015

Did the auto-complete item window change from "normal" repeat to "virtualRepeat"? from 10.1 to 11? and if so is there a way to force non-virtualRepeat?

@bluebirdtech
Copy link
Author

Feel free to commit the relevant change in the PR if you can find them easily. Need to learn how to do a PR correctly. :)

@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc1, 1.0-rc2 Oct 27, 2015
@bluebirdtech
Copy link
Author

I've been using the following fix without problems:
bluebirdtech@0554b1d

I would submit a PR, but I was declined permission.

@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc2, 1.0-rc3 Oct 30, 2015
@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc3, 1.0-rc4 Nov 7, 2015
@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc4, 1.0-rc5 Nov 16, 2015
@kseamon
Copy link
Contributor

kseamon commented Nov 25, 2015

Unless I'm misunderstanding the bug, it seems like this is working for me on HEAD (Fixed by #5908?).

@bluebirdtech Can you confirm?

@ThomasBurleson ThomasBurleson modified the milestones: 1.0-rc5, 1.0-rc6 Nov 25, 2015
@jelbourn
Copy link
Member

@bluebirdtech Closing this issue now, let us know if there's still a problem.

@tonyawad88
Copy link

I am using 1.0.0 RC4 and I am still getting it.

<div class="row row-items srch-bar" ng-class="{'p-r-60':!isnative,'p-l-60':isnative}">

    <md-chips ng-model="selectedTags" md-autocomplete-snap md-require-match="true">
        <md-autocomplete md-autoselect="true"
                         md-selected-item="selectedItem"
                         md-search-text="searchText"
                         md-items="item in querySearch(searchText)"
                         md-item-text="item.hashtag_val"
                         placeholder="{{lblSrchInput}}"
                         md-select-on-match="false">
            <md-item-template>
                <span md-highlight-text="searchText" md-highlight-flags="^i">{{item.name}}</span>
            </md-item-template>
        </md-autocomplete>
        <md-chip-template>

          {{$chip.name || $chip}}

        </md-chip-template>
    </md-chips>

    <md-button class="srch-btn-cstm" ng-class="{'push-left-0':isnative,'push-right-0':!isnative}"
               ng-disabled="looking" ng-click="Search();" aria-label="srchbtn">
        <md-icon class="s64 icons-header fa fgdarkgrey" ng-class="{'fa-search':!looking,
                                                                   'fa-spin fa-cog':looking} " />
    </md-button>
</div>

screen shot 2015-11-30 at 6 55 54 pm

@jelbourn
Copy link
Member

jelbourn commented Dec 1, 2015

@tonyawad88 Do you see the problem with RC5?

@tonyawad88
Copy link

@jelbourn It is still a bug in RC5. I just tried it.
when I scroll, it only shows 3 items although there's more in the list, as per below:

part3

@jelbourn jelbourn reopened this Dec 1, 2015
@tonyawad88
Copy link

@jelbourn I might have found something. In virtualRepeat.js, there's a variable NUM_EXTRA. If we change it from 3 to 8, it seems to fix part of the problem. When there are 5 or more items in the suggestion list, they are displayed properly. But when it's less than 5 it still doesn't autoshrink.

/**
 * Number of additional elements to render above and below the visible area inside
 * of the virtual repeat container. A higher number results in less flicker when scrolling
 * very quickly in Safari, but comes with a higher rendering and dirty-checking cost.
 * @const {number}
 */
var NUM_EXTRA = 8;

screen shot 2015-11-30 at 9 30 01 pm

screen shot 2015-11-30 at 9 30 13 pm

@kseamon kseamon modified the milestones: 1.0-rc7, 1.0-rc6 Dec 1, 2015
@MigFerreira
Copy link

@jelbourn Was having the same issue.
Fixed it by cleaning the innerHtml when sizing the scroller.

VirtualRepeatContainerController.prototype.sizeScroller_ = function(size) {
  var dimension =  this.isHorizontal() ? 'width' : 'height';
  var crossDimension = this.isHorizontal() ? 'height' : 'width';

  // If the size falls within the browser's maximum explicit size for a single element, we can
  // set the size and be done. Otherwise, we have to create children that add up the the desired
  // size.
  if (size < MAX_ELEMENT_SIZE) {
    this.sizer.innerHTML = '';
    this.sizer.style[dimension] = size + 'px';
  }
...

@tonyawad88
Copy link

@MigFerreira I tried just adding that portion:

if (size < MAX_ELEMENT_SIZE) {
    this.sizer.innerHTML = '';
    this.sizer.style[dimension] = size + 'px';
  }

It didn't help... I'm using v1.0.0 RC6
part3

@MigFerreira
Copy link

@tonyawad88 in my case the bug was using directly virtual repeat and not autocomplete.
Although the effect was exactly like yours.

I tried recreating your case in codepen but could get the issue. If you could create a pen i will gladly help you out.

@tonyawad88
Copy link

@bluebirdtech Have you found any workarounds for this ? The virtual repeat container doesn't autoshrink when the number of items have a height less than the original max-height passed by the autocomplete:

var ITEM_HEIGHT   = 41,
    MAX_HEIGHT    = 5.5 * ITEM_HEIGHT,
    ...

Is this the bug you've been having ? Thanks

@tonyawad88
Copy link

@MigFerreira @ThomasBurleson @kseamon @jelbourn I found what is causing this issue. It's quite interesting.
Here's the codepen with the problem: http://codepen.io/tonyawad88/pen/OMJpON/
Here's the codepen with the solution: http://codepen.io/tonyawad88/pen/Vewbpj

Problem:
The way I have angular material setup, is that I include modules as I need them. So I had these included:

    "material.core",
    "material.components.autocomplete",
    "material.components.virtualRepeat",
    "material.components.chips",
    ...

Solution
I tried replicating on codepen and included all by using "ngMaterial" and it worked.

Any idea what the root of this problem might be ? Do I have to include the modules in a certain order or am I missing a component that I should include? I have no warnings or errors for the missing component.

Please let me know if you need more info.

@tonyawad88
Copy link

@ThomasBurleson @kseamon @jelbourn I promise this is my last post on this, sorry to bug you all. I was able to narrow it down to the root cause.

Component virtualRepeat depends on component showHide. We should be throwing an error when "showHide" component is not included. Currently virtualRepeat only depends on material.core.

Suggested fix to virtualRepeat.js:

angular.module('material.components.virtualRepeat', [
  'material.core',
  "material.components.showHide",
])

I haven't done a PR before. If you send me some documents/guidance, I can help you guys out.
Cheers.

@jelbourn
Copy link
Member

jelbourn commented Dec 4, 2015

@tonyawad88 Thanks for the debugging! I submitted a change with your fix.

Anyone else let us know if this issue still occurs.

@MigFerreira
Copy link

@jelbourn the scrolling issue visble in the images above still occur when you have more than MAX_ELEMENT_SIZE elements then filter and get less.

In this case when the size is updated the innerHTML must also be cleaned out.

if (size < MAX_ELEMENT_SIZE) {
    this.sizer.innerHTML = '';
    this.sizer.style[dimension] = size + 'px';
}

@jelbourn
Copy link
Member

jelbourn commented Dec 4, 2015

@MigFerreira good point. Created fix in #6087

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

No branches or pull requests

8 participants