Conversation
…strap to give problems
|
While making the changes, I was tempted to add some data within the seeds to further test and edit the UI, but realized that is out of scope. So, I wanted to put in a request add more seeds for items, wishlist_items and some tests to fully flesh out functionality. |
|
@mcshakes That's a good idea; I made an issue (#157), and if we don't get a PR for it shortly, I'll make the change myself. For now, I went onto your review app and added a bunch of items; that'll show you the current state of your PR. You can add items locally too by doing something similar to this in wishlist_item = WishlistItem.last
50.times do |n|
new_item = wishlist_item.dup
new_item.staff_message = "This is newly-added item ##{n+1}."
new_item.save!
endThat'll add 50 new corgi plushes to your local app. |
|
@leesharma Sweet! Taking another look to correct some of the UI before pushing again |
…s' to 'Prev'. I may even get rid of that if need be. Looks busy
|
@leesharma Ready to be merged, but looking at the conflicts; are those changes ones you wanted to keep? |
invacuo
left a comment
invacuo
left a comment
There was a problem hiding this comment.
Just a couple of minor nits.
Can you push it up to the staging box again? Or include screenshots?
Thanks for your help!
| def index | ||
| skip_authorization | ||
| @wishlist_items = WishlistItem.includes(:item, :wishlist).priority_order | ||
| @wishlist_items = WishlistItem.includes(:item, :wishlist).page(params[:page]).per(10) |
There was a problem hiding this comment.
Will this work if we specify default_per_page = 10 in the config file and remove .per(10)?
There was a problem hiding this comment.
Removed the .per(10) and using the config.yml instead for this
| <%= first_page_tag unless current_page.first? %> | ||
| <%= prev_page_tag unless current_page.first? %> | ||
| <% each_page do |page| %> | ||
| <% if page.left_outer? || page.right_outer? || page.inside_window? %> |
There was a problem hiding this comment.
This is one of the auto generated files, right?
The indentation looks a little off. Is it possible to fix it?
There was a problem hiding this comment.
Yes, it is. It looks to be centered to the content div, but let me check and play around with it.
There was a problem hiding this comment.
So, I'm pretty sure it is horizontally centered. I did add some top margins so it's uniform to the bottom and not hugging the line below the item
There was a problem hiding this comment.
Sorry, I was referring to code indentation not the alignment.
There was a problem hiding this comment.
Gotcha, my mistake. Fixed with latest push
…age method within config.
…/playtime into paginate-wishlist-items
4 participants
Resolves #141
Description
Adds pagination.
Type of change