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

Impression measurement config #1

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

prachi-myntra
Copy link
Collaborator

Adding viewability config to add support for minimum item view time and minimum item view percentage.

const timeoutId = setTimeout(() => {
that.timers.delete(timeoutId);

const currAll = all.filter((index) => that._visibleIndexes.indexOf(index) >= 0);

Choose a reason for hiding this comment

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

What is the max array batch size of all: number[]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all has all the list items that are visible in viewport at a time
So it depends on list items sizes

@@ -181,7 +188,7 @@ export default class RecyclerListView<P extends RecyclerListViewProps, S extends
this._pendingScrollToOffset = offset;
}, (index) => {
return this.props.dataProvider.getStableId(index);
}, !props.disableRecycling);
}, !props.disableRecycling, props.viewabilityConfig);

Choose a reason for hiding this comment

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

viewabilityConfig is just initialized in the constructor any changes to viewabilityConfig after the first render will not be respected.
This use case would come for pagination later with multiple impression configs at the widget level.

let visibleItemContent = 0;
const itemSize = itemEndBound - itemStartBound;

if (window.start >= itemStartBound && window.end >= itemEndBound) {

Choose a reason for hiding this comment

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

can you add some comments explaining the if-else with some examples

if (func) {
const now = this._calculateArrayDiff(newItems, oldItems);
const notNow = this._calculateArrayDiff(oldItems, newItems);
if (now.length > 0 || notNow.length > 0) {
func([...newItems], now, notNow);
// Adding default minimum view time of 250ms for performance optimization
if (minimumViewTime && minimumViewTime >= Constants.DEFAULT_MIN_VIEW_TIME) {

Choose a reason for hiding this comment

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

instead of not executing the minimumViewTime logic incase of DEFAULT_MIN_VIEW_TIME greater than minimumViewTime you could execute with minimumViewTime logic with default value.

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.

5 participants