GrooveOtter / grooveotter-client

Front-end client for GrooveOtter
http://grooveotter-demo.herokuapp.com/
0 stars 0 forks source link

fixed repeated people in feed #253

Closed garrethdev closed 9 years ago

garrethdev commented 9 years ago

@Arilas @devicodev Currently on this pr request, there's an issue of multiple repeating items. The first task of the day and a shared task will show back to back. There should never be an incident where the newsfeed shows the same user twice in a row.

Ryan-Parker commented 9 years ago

@Arilas There seems to be an issue now when the newsfeed rotates it will expand out before landing on the intended shared task. This happens infrequently, but does happen.

screen shot 2015-09-10 at 8 05 12 pm

Arilas commented 9 years ago

Removed usage of TransitionGroup, changes logic of newsfeed shuffle

Ryan-Parker commented 9 years ago

@Arilas Removing of that now make the newsfeed flash when someone attempts to click the like button.

Arilas commented 9 years ago

Fixed Conflicts

Arilas commented 9 years ago

@Ryan-Parker finally fixed this

garrethdev commented 9 years ago

@Arilas the issue I'm still seeing is that when you post two items the second item is thrown near the end of the collection. Please paste the code you're using to do this. As we want it to be near the front of the collection. Otherwise relevant posts are never seen.

Arilas commented 9 years ago

So, I've have this check:

        //Check user of current Notification and Previous
        if (currentItem.get('user_id') == this.previousItem.get('user_id') && currentItemIndex < newsfeed.length - 2) {
            var canGo = false
            //Check for prevent Recursion, we check for available another users in collection after current
            this.newsfeed.each(function(notification, index) {
                if (index > currentItemIndex && notification.get('user_id') != this.previousItem.get('user_id')) {
                    canGo = true
                }
            }.bind(this))
            //If we have another user in collection, that we can display
            if (canGo) {
                //removing notification from current position
                this.newsfeed.remove(currentItem)
                //placing notification to the end of collection
                this.newsfeed.add(currentItem, {at: this.newsfeed.length - 1})
                this.onCycleNewsfeed()
            } else {
                //There's more than 2 notifications in the end of newsfeed with the same user, so we reset current Index
                this.currentItemIndex = 0
                this.previousItem = null
                this.onCycleNewsfeed()
            }
            return
        }

So as you can see, it's prevent situations when we repeat the same user twice in a row. Yes, it's paste notification to the end of collection as we discuss.

Previously I've used shuffle, to prevent this.

Ryan-Parker commented 9 years ago

@Arilas What we discussed this morning was not to have it paste to the end of the collection but instead move the task one over if the system identifies there is a repeat.

garrethdev commented 9 years ago

@Arilas this is what Im envisioning. Its a simple system the only edge case is when the last two items are the same. In that case you just setup another if statement to see if its the last two items in the list, if thats the case then break out of the loop.

var testArray = [1,1,2,3,4,5,6,7,9,9] var newArray = testArray.map(function(item,index){ if(testArray[index] == testArray[index+1]) { var tempValue = testArray[index+1]; var secondTemp = testArray[index+2]; testArray[index+1] = secondTemp; testArray[index+2] = tempValue; } return item;

})

garrethdev commented 9 years ago

@Arilas this looks good, for future reference though lets just make sure to comment on the ticket @Ryan-Parker as soon as its done that way we can get feedback quicker. But nice job with this one.