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

Added: refresh() function now accepts optional 'data' parameter which ca... #32

Merged
merged 4 commits into from
Mar 25, 2015

Conversation

margru
Copy link
Contributor

@margru margru commented Feb 10, 2015

Added: refresh() function now accepts optional 'data' parameter which can affect the way the items are scrolled in the view.

I added a functionality that I needed for my project. If you find it usefull also for you or the others, merge it into.

The functionality enhances the current behaviour. The user can trigger the 'vsRepeatTrigger' with additional data - an object with the following properties:

  • startIndex - index of the item that should be considered as the first item; it is not necessarily the one that will be rendered at the first position, the first rendered item depends also on the startIndexPosition;
  • startIndexPosition
    • position of the first item, i.e. how much it should be scrolled; if not provided, no scrolling is performed;
    • either a number of pixels or one of the following strings: 'top', 'middle', 'bottom', 'inview' is the same as 'inview#top', 'inview#middle', 'inview#bottom', 'inview#auto';
    • the 'inview#' settings means that if the item is already in the view, nothing is scrolled, but if it is not, then the item will be scrolled accordingly (to be in the );
    • position 'auto' means that it will be either 'top' or 'bottom' depending on what is closer to the current item position;

… can affect the way the items are scrolled in the view.
@kamilkp
Copy link
Owner

kamilkp commented Feb 12, 2015

Nice job. If i remember correclty, there were several people requesting a functionality like that. I saw some (commented out), can you remove them, and also copy the description of how your functionality works and paste it into the readme.md? i'll definitely merge it then, thanks.

@margru
Copy link
Contributor Author

margru commented Feb 13, 2015

OK, I'll improve the readme and cleanup the code. I was also thinking about integrating the idea of #8, i.e. that the startIndex and startIndexPosition could be also binded to directive's attributes. A user would then have two possibilities to affect the behaviour.

…g the way the items are scrolled in the view.

Updated: README.md
Comments cleanup.
@margru
Copy link
Contributor Author

margru commented Mar 24, 2015

Well, it took some time since I was busy but I updated the readme and I also incorporated a mechanism similar to the #8, i.e. the scrolling can be controlled also via directive attributes.

I just renamed the settings: startIndex -> scrollIndex, startIndexPosition -> scrollIndexPosition.

@kamilkp
Copy link
Owner

kamilkp commented Mar 25, 2015

Great. One more thing. Could you not wrap my one-statement ifs with curly braces, change double quotes to single quotes and all that? I'd like to have changes that only affect the functionality that you implemented

@margru
Copy link
Contributor Author

margru commented Mar 25, 2015

Done ;)

kamilkp added a commit that referenced this pull request Mar 25, 2015
Added: refresh() function now accepts optional 'data' parameter which ca...
@kamilkp kamilkp merged commit 21138a9 into kamilkp:master Mar 25, 2015
@kamilkp
Copy link
Owner

kamilkp commented May 15, 2015

There is is a bug #43 with the merged functionality. Is that an easy fix?

@margru
Copy link
Contributor Author

margru commented May 15, 2015

Fixed in #44.

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