gnocode / noladex.org

People of New Orleans
http://www.noladex.org
21 stars 10 forks source link

Endless pageless for the users page ... #44

Closed matthewling closed 12 years ago

matthewling commented 12 years ago

Hey there, hacked this together. It may be worth a look.

I'm pretty sure that the Users#index can be improved for performance. but I went for the most clear thing to read first. I have some ideas to pull the counts into the model and return them from there, as the model code could probably count the records fairly simply too.

RIght now, testing with 150 odd records does improve the initial page load time. But I think the possible big win is that the rest of the assets won't hassle the browser if you're not scrolling ... Would love to know how this performs against some production data...

@joeellis maybe if you get a chance to run this code against something close to production and let me know how it was ... ?

Have a good weekend lads ...

matthewling commented 12 years ago

PS! I tested on Safari, Chrome, FF, iPhone and iPad ... the only bug was the footer bar on the iPad seemed to not move with the page ... I didn't touch the styling of the loading message either, maybe thats for someone who knows better ... :)

I also checked in IE7 & 8 but I think there's something larger going on there ...

Testing on Android and other devices required!!! :)

fholgado commented 12 years ago

iOS 4 doesn't have position: absolute; support. It's coming in iOS 5, so that should be ok!

matthewling commented 12 years ago

Ahh .. nice one! Good to know ...

joeellis commented 12 years ago

I'll see if I can take a look at this tomorrow at the Hack Night. I also think Matt Shwery was thinking of redoing the layout to get rid of the footer bar anyways, so it's probably not going to be an issue. Thanks for the patch!

matthewling commented 12 years ago

Cool. Due to the randomness of the users being shown, it's a bit tricky to page, so the User#index kind of does a select * on each request while excluding already displayed users.

This isn't brilliant, but should be ok for the time being considering there are a reasonable amount of users. I can imagine caching the user ids in an application var which could be used to fetch a random page of results. I can make this change in the coming days, but maybe it's fun to see how it works already, as the primary key index of the users table should mean that paging like that won't mean the world.

matthewling commented 12 years ago

Hey guys ... I think I nearly got the big performance problem cracked ... maybe don't waste your time checking just yet ...

matthewling commented 12 years ago

Ok, so this is much faster. I have the paging tomes down to about ~200 ms on my macbook pro. Let me know how you get on ...

matthewling commented 12 years ago

PS I included a cucumber test, but I didn't see it that far through as the randomness of the results made the testing a little bonkers ... will have to think on that a little more ... just run cucumber from the command line after installing all gems ...

mshwery commented 12 years ago

Can't do a smooth merge on this, unfortunately. Did you say you were potentially refactoring this anyway?

matthewling commented 12 years ago

Well, I would convert the haml to erb to make sure that it's inline with the standard template language, if you want I could rebase the project, make sure it works with the current version ... and then maybe advise on how to merge the parts?

matthewling commented 12 years ago

Hi Guys, I cleaned up the master branch of my fork. I then merged from your master and got everything working. Basically, I removed all the cucumber stuff, cause maybe its too much hassle right now for that, and converted all haml to erb to be inline with the templating.

So the important parts to resolve any merge conflicts are this:

The UsersController#index can be taken over 1:1 views/users/index.html.erb is now in erb and has some javascript which will be injected into the application.html.erb application.html.erb has 2 new things, the query.pageless.js and the yield javascript down the end of the file. user.rb has 2 new parts, the attributes stuff up the top and the find_by_category method is replaced by get_page application.rb will be fine, has just a new page size config var seeds.rb seeds with 27 items for testing Gemfile adds will paginate gem

and then there are a few resources in public for the page less, but that should just merge ...

Let me know if I can help more ...

mshwery commented 12 years ago

Seems to work well.

I think the number of items loaded should match a certain number of full rows. Is there a way to determine how many columns there are (it's variable based on the window size, because all the items are floated left), and then just load enough to fill the next 3-4 rows fully?

Note: picture dimensions may be reduced in the next layout.

matthewling commented 12 years ago

Hey Matt, well the pagesize is configurable, right now I think its around 9 items, so it loads in rows of 3 on a desktop or 9 on an iphone ... , or should do. It's probably possible to vary that depending on the viewport size, but maybe just making the page size smaller would yield better results.

I think it should also still work with smaller pictures, but then the page size could probably be made larger so that the first load has enough to fill a large screen, but the image size shouldnt influence things massively, as the viewport size is whats calculated for the next pageload ...

mshwery commented 12 years ago

Matthew - can you pull down the "test" branch to find an error? For some reason, the number of users loaded by the end of the page is far fewer than the actual number of users. Perhaps you can figure out where the discrepancy is in the never-ending scroll...

matthewling commented 12 years ago

Hey Matt, I can, it might a bit tho as we're under pressure, I'll have a look and let you know asap ...

mshwery commented 12 years ago

no rush.

Matt

On Thu, Sep 8, 2011 at 1:55 PM, Matthew Ling < reply@reply.github.com>wrote:

Hey Matt, I can, it might a bit tho as we're under pressure, I'll have a look and let you know asap ...

Reply to this email directly or view it on GitHub: https://github.com/gnocode/noladex.org/pull/44#issuecomment-2043280

matthewling commented 12 years ago

Cool ;)