Skip to content
This repository was archived by the owner on Jan 1, 2020. It is now read-only.

Conversation

@meh-uk
Copy link
Contributor

@meh-uk meh-uk commented Jan 6, 2015

We had a bug where if you scrolled slowly through the grid in infinite scroll mode with server side data that the database wasn't displayed correctly, and we found that this change fixed the problem.

@OlegKi do you have a view on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously this should be done by removing the whole if block.

@OlegKi
Copy link
Contributor

OlegKi commented Jan 8, 2015

I think that virtual scrolling is a feature which need be changed more deep or be rewritten. The suggested changes are inside of populateVisible function which should be called only in case of usage virtual scrolling (in both kind of virtual scrolling: scroll: 1 and scroll: true). So it's difficult for me to comment the changes.

I develop personally jqGrid in my repository only. One from the reason: I can fix all problems which I know without any additional discussions with the colleges. In case of usage common repository, used by multiple people, one should have clear parts of code (or unit tests, build process or other part of github repository) where one person is responsible for merging. If another person have a suggestion it would be important to hold some rules. One should first prepare small demo which can be used to reproduce the problem which one try to solve (the demo which is really slow) and another one which uses the fixed code and which works quickly.

As an example you can look at the post. The description of the problem includes two demos which I mention above. The pull request includes some additional descriptions.

Only in the way one can really be sure that suggested changes really helpful.

It would be nice to make additional tests which shows that the changes have no side effects, but it will be impossible to do till we have good unit tests.

Best regards
Oleg

@meh-uk meh-uk added this to the 2.0 milestone Jan 12, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants