Compare commits

...

2 commits

Author SHA1 Message Date
583f3c712f High-level caching for closet lists
Okay, so I still don't know why rendering is just so slow (though
migrating away from item translations did help!), but I can at least
cache entire closet lists as a basic measure.

That way, the first user to see the latest version of a closet list
will still need just as much time to load it… but *only* the ones that
have changed since last time (rather than always the full page), and
then subsequent users get to reuse it too!

Should help a lot for high-traffic lists, which incidentally are likely
to be the big ones belonging to highly active traders!

One big change we needed to make was to extract the `user-owns` and
`user-wants` classes (which we use for trade matches for *the user
viewing the list right now*) out of the cached HTML, and apply them
after with Javascript instead. I always dislike moving stuff to JS, but
the wins here seem. truly very very good, all things considered!
2024-02-20 18:43:39 -08:00
13b92b30d0 Replace old stickUp dependency with position: sticky
From an era when we didn't have that! Now we do!

(My motivation is that I'm trying to add new JS to this page and errors
in stickUp are crashing the page early, womp womp!)
2024-02-20 18:33:23 -08:00
10 changed files with 147 additions and 95 deletions

View file

@ -7,7 +7,11 @@
function hangersInit() {
for (var i = 0; i < hangersInitCallbacks.length; i++) {
hangersInitCallbacks[i]();
try {
hangersInitCallbacks[i]();
} catch (error) {
console.error(error);
}
}
}
@ -58,6 +62,36 @@
hangersEl.toggleClass("comparing");
});
// Read the item IDs of trade matches from the meta tags.
const ownedIds =
document
.querySelector("meta[name=trade-matches-owns]")
?.getAttribute("value")
?.split(",") ?? [];
const wantedIds =
document
.querySelector("meta[name=trade-matches-wants]")
?.getAttribute("value")
?.split(",") ?? [];
// Apply the `user-owns` and `user-wants` classes to the relevant entries.
// This both provides immediate visual feedback, and sets up "Compare with
// Your Items" to toggle to just them!
//
// NOTE: The motivation here is caching: this allows us to share a cache of
// the closet list contents across all users, without `user-owns` or
// `user-wants` classes for one specific user getting cached and reused.
const hangerEls = document.querySelectorAll("#closet-hangers .object");
for (const hangerEl of hangerEls) {
const itemId = hangerEl.getAttribute("data-item-id");
if (ownedIds.includes(itemId)) {
hangerEl.classList.add("user-owns");
}
if (wantedIds.includes(itemId)) {
hangerEl.classList.add("user-wants");
}
}
/*
Hanger forms
@ -481,13 +515,6 @@
updateBulkActions();
});
// hahaha, nasty hacks to make stickUp use our old jQuery
$.fn.on = $.fn.bind;
$(function () {
$(".bulk-actions").stickUp();
});
function maintainCheckboxes(fn) {
var checkedIds = getCheckedIds();

View file

@ -1 +0,0 @@
jQuery(function($){$(document).ready(function(){var contentButton = [];var contentTop = [];var content = [];var lastScrollTop = 0;var scrollDir = '';var itemClass = '';var itemHover = '';var menuSize = null;var stickyHeight = 0;var stickyMarginB = 0;var currentMarginT = 0;var topMargin = 0;$(window).scroll(function(event){var st = $(this).scrollTop();if (st > lastScrollTop){scrollDir = 'down';} else {scrollDir = 'up';}lastScrollTop = st;});$.fn.stickUp = function( options ) {$(this).addClass('stuckMenu');var objn = 0;if(options != null) {for(var o in options.parts) {if (options.parts.hasOwnProperty(o)){content[objn] = options.parts[objn];objn++;}}if(objn == 0) {console.log('error:needs arguments');}itemClass = options.itemClass;itemHover = options.itemHover;if(options.topMargin != null) {if(options.topMargin == 'auto') {topMargin = parseInt($('.stuckMenu').css('margin-top'));} else {if(isNaN(options.topMargin) && options.topMargin.search("px") > 0){topMargin = parseInt(options.topMargin.replace("px",""));} else if(!isNaN(parseInt(options.topMargin))) {topMargin = parseInt(options.topMargin);} else {console.log("incorrect argument, ignored.");topMargin = 0;} }} else {topMargin = 0;}menuSize = $('.'+itemClass).size();}stickyHeight = parseInt($(this).height());stickyMarginB = parseInt($(this).css('margin-bottom'));currentMarginT = parseInt($(this).next().closest('div').css('margin-top'));vartop = parseInt($(this).offset().top);};$(document).on('scroll', function() {varscroll = parseInt($(document).scrollTop());if(menuSize != null){for(var i=0;i < menuSize;i++){contentTop[i] = $('#'+content[i]+'').offset().top;function bottomView(i) {contentView = $('#'+content[i]+'').height()*.4;testView = contentTop[i] - contentView;if(varscroll > testView){$('.'+itemClass).removeClass(itemHover);$('.'+itemClass+':eq('+i+')').addClass(itemHover);} else if(varscroll < 50){$('.'+itemClass).removeClass(itemHover);$('.'+itemClass+':eq(0)').addClass(itemHover);}}if(scrollDir == 'down' && varscroll > contentTop[i]-50 && varscroll < contentTop[i]+50) {$('.'+itemClass).removeClass(itemHover);$('.'+itemClass+':eq('+i+')').addClass(itemHover);}if(scrollDir == 'up') {bottomView(i);}}}if(vartop < varscroll + topMargin){$('.stuckMenu').addClass('isStuck');$('.stuckMenu').next().closest('div').css({'margin-top': stickyHeight + stickyMarginB + currentMarginT + 'px'}, 10);$('.stuckMenu').css("position","fixed");$('.isStuck').css({top: '0px'}, 10, function(){});};if(varscroll + topMargin < vartop){$('.stuckMenu').removeClass('isStuck');$('.stuckMenu').next().closest('div').css({'margin-top': currentMarginT + 'px'}, 10);$('.stuckMenu').css("position","relative");};});});});

View file

@ -96,6 +96,7 @@ body.closet_hangers-index
#closet-hangers
clear: both
text-align: center
border-top: 1px solid $module-border-color
.object
.quantity
@ -152,6 +153,9 @@ body.closet_hangers-index
margin-bottom: 2em
padding-bottom: 1em
&:first-of-type
border-top: 0
> header
border-bottom: 1px solid $soft-border-color
display: block
@ -376,7 +380,6 @@ body.closet_hangers-index
.bulk-actions
background: $background-color
border-top: 1px solid $module-border-color
box-sizing: border-box
display: block
font-size: 85%
@ -384,10 +387,10 @@ body.closet_hangers-index
text-align: center
width: 800px
&.isStuck
border-bottom: 1px solid $module-border-color
border-top: 0
z-index: 11 /* beat the visibility form */
position: sticky
top: 0
border-bottom: 1px solid $module-border-color
z-index: 11 /* beat the visibility form */
.bulk-actions-intro, .bulk-actions-target-desc-singular
display: none

View file

@ -45,24 +45,24 @@ class ClosetHangersController < ApplicationController
visible_groups = @user.closet_hangers_groups_visible_to(@perspective_user)
@unlisted_closet_hangers_by_owned = find_unlisted_closet_hangers_by_owned(visible_groups)
items = []
@items = []
@closet_lists_by_owned.each do |owned, lists|
lists.each do |list|
list.hangers.each do |hanger|
items << hanger.item
@items << hanger.item
end
end
end
@unlisted_closet_hangers_by_owned.each do |owned, hangers|
hangers.each do |hanger|
items << hanger.item
@items << hanger.item
end
end
if @public_perspective && user_signed_in?
current_user.assign_closeted_to_items!(items)
current_user.assign_closeted_to_items!(@items)
end
@campaign = Fundraising::Campaign.current

View file

@ -1,17 +1,11 @@
require 'cgi'
require 'digest'
module ClosetHangersHelper
def closet_hangers_help_class(user)
'hidden' unless user.closet_hangers.empty?
end
def closet_hanger_partial_class(hanger)
'object'.tap do |class_name|
class_name << ' user-owns' if hanger.item.owned?
class_name << ' user-wants' if hanger.item.wanted?
end
end
def send_neomail_url(neopets_username)
"https://www.neopets.com/neomessages.phtml?type=send&recipient=#{CGI.escape neopets_username}"
end
@ -124,8 +118,8 @@ module ClosetHangersHelper
def render_closet_lists(lists)
if lists
render :partial => 'closet_lists/closet_list', :collection => lists,
:locals => {:show_controls => !public_perspective?}
render partial: 'closet_lists/closet_list', collection: lists,
locals: {show_controls: !public_perspective?}
end
end
@ -139,6 +133,20 @@ module ClosetHangersHelper
hangers = @unlisted_closet_hangers_by_owned[owned]
hangers ? hangers.size : 0
end
def unlisted_hangers_cache_key(owned)
# It'd be nice if this was a closet list like any other, instead of being a
# weird separate thing! Then we could use the same closet list cache key,
# in which adding/removing stuff in the list touches the simple updated_at.
# Instead, we encode the hashed list of hanger IDs and last updated_at into
# the key! (The digest is to prevent the cache key from getting obscenely
# large for long lists!)
hangers = @unlisted_closet_hangers_by_owned[owned] || []
ids = hangers.map(&:id).join(",")
last_updated_at = hangers.map(&:updated_at).max
["owned=#{owned}", Digest::MD5.hexdigest(ids), last_updated_at.to_i]
end
def closet_lists_group_name(subject, owned)
ownership_key = owned ? 'owned_by' : 'wanted_by'

View file

@ -1,6 +1,6 @@
class ClosetHanger < ApplicationRecord
belongs_to :item
belongs_to :list, class_name: 'ClosetList', optional: true
belongs_to :list, class_name: 'ClosetList', optional: true, touch: true
belongs_to :user
delegate :name, to: :item, prefix: true

View file

@ -1,4 +1,5 @@
%div{'class' => closet_hanger_partial_class(closet_hanger), 'data-item-id' => closet_hanger.item_id, 'data-quantity' => closet_hanger.quantity, 'data-id' => closet_hanger.id}
-# NOTE: Changing this requires bumping the cache at `_closet_list.html.haml`!
.object{'data-item-id' => closet_hanger.item_id, 'data-quantity' => closet_hanger.quantity, 'data-id' => closet_hanger.id}
= render_item_link(closet_hanger.item)
.quantity{:class => "quantity-#{closet_hanger.quantity}"}
%span= closet_hanger.quantity

View file

@ -55,34 +55,33 @@
= link_to t('.import_from.gallery'), new_neopets_page_import_task_path(page_type: 'gallery', expected_index: 1)
= link_to t('.export_to.petpage'), petpage_user_closet_hangers_path(@user)
- unless public_perspective?
-# TODO: i18n
.bulk-actions{'data-target-count' => 0}
.bulk-actions-intro
Manage items in bulk! Select an item by clicking its thumbnail, or choose
a list and Select All.
.bulk-actions-form
.bulk-actions-target-desc
%span.bulk-actions-target-desc-singular
With the 1 selected item:
%span.bulk-actions-target-desc-plural
With the
%span.bulk-actions-target-count 0
selected items:
%ul.bulk-actions-options
%li
= form_tag user_closet_hangers_path(@user), method: :put, class: 'bulk-actions-move-all' do
= select_tag 'list_id', options_for_select(destination_options)
%button Move
%li
= form_tag user_closet_hangers_path(@user), method: :delete, class: 'bulk-actions-remove-all' do
%button Remove all
%li
%button.bulk-actions-deselect-all Deselect all
#closet-hangers{:class => public_perspective? ? nil : 'current-user'}
- unless public_perspective?
-# TODO: i18n
.bulk-actions{'data-target-count' => 0}
.bulk-actions-intro
Manage items in bulk! Select an item by clicking its thumbnail, or choose
a list and Select All.
.bulk-actions-form
.bulk-actions-target-desc
%span.bulk-actions-target-desc-singular
With the 1 selected item:
%span.bulk-actions-target-desc-plural
With the
%span.bulk-actions-target-count 0
selected items:
%ul.bulk-actions-options
%li
= form_tag user_closet_hangers_path(@user), method: :put, class: 'bulk-actions-move-all' do
= select_tag 'list_id', options_for_select(destination_options)
%button Move
%li
= form_tag user_closet_hangers_path(@user), method: :delete, class: 'bulk-actions-remove-all' do
%button Remove all
%li
%button.bulk-actions-deselect-all Deselect all
- [true, false].each do |owned|
.closet-hangers-group{'data-owned' => owned.to_s, :id => "closet-hangers-group-#{owned}"}
%section.closet-hangers-group{'data-owned' => owned.to_s, :id => "closet-hangers-group-#{owned}"}
%header
%h3= closet_lists_group_name(closet_hangers_subject(@user), owned)
%span.toggle.show= t '.toggle_group.show'
@ -109,44 +108,44 @@
%h4= t '.unlisted.header'
- if !@public_perspective
= render partial: 'closet_lists/trading_neomail_warning', locals: {list: @user.null_closet_list(owned), user: @user}
.closet-list-content
.closet-list-hangers
= render_unlisted_closet_hangers(owned)
%span.empty-list= t '.unlisted.empty'
- cache unlisted_hangers_cache_key(owned) do
.closet-list-content
.closet-list-hangers
= render_unlisted_closet_hangers(owned)
%span.empty-list= t '.unlisted.empty'
- if user_signed_in?
- localized_cache action_suffix: 'tmpls' do
%script#autocomplete-item-tmpl{type: 'text/x-jquery-tmpl'}
%a
= t '.autocomplete.add_item_html', item_name: '${item_name}'
%script#autocomplete-item-tmpl{type: 'text/x-jquery-tmpl'}
%a
= t '.autocomplete.add_item_html', item_name: '${item_name}'
%script#autocomplete-add-to-list-tmpl{type: 'text/x-jquery-tmpl'}
%a
= t '.autocomplete.add_to_list_html', list_name: '${list_name}'
%script#autocomplete-add-to-list-tmpl{type: 'text/x-jquery-tmpl'}
%a
= t '.autocomplete.add_to_list_html', list_name: '${list_name}'
%script#autocomplete-add-to-group-tmpl{type: 'text/x-jquery-tmpl'}
%a
= t '.autocomplete.add_to_group_html', group_name: '${group_name}'
%script#autocomplete-add-to-group-tmpl{type: 'text/x-jquery-tmpl'}
%a
= t '.autocomplete.add_to_group_html', group_name: '${group_name}'
%script#autocomplete-already-in-collection-tmpl{type: 'text/x-jquery-tmpl'}
%span
= t '.autocomplete.already_in_collection_html',
collection_name: '${collection_name}'
%script#autocomplete-already-in-collection-tmpl{type: 'text/x-jquery-tmpl'}
%span
= t '.autocomplete.already_in_collection_html',
collection_name: '${collection_name}'
-# Gotta do this weird subbing in the path, since the braces will be
-# escaped if they themselves are inserted. Probably deserves a more legit
-# method, especially if we ever need it again.
- templated_hanger_path = user_closet_hanger_path(user_id: '$0', id: '$1').sub('$0', '${user_id}').sub('$1', '${closet_hanger_id}')
%script#closet-hanger-update-tmpl{type: 'text/x-jquery-tmpl'}
= form_tag templated_hanger_path, method: :put, authenticity_token: false, class: 'closet-hanger-update' do
= hidden_field_tag 'closet_hanger[list_id]', '${list_id}'
= hidden_field_tag 'closet_hanger[owned]', '${owned}'
= number_field_tag 'closet_hanger[quantity]', '${quantity}',
min: 0, required: true
-# Gotta do this weird subbing in the path, since the braces will be
-# escaped if they themselves are inserted. Probably deserves a more legit
-# method, especially if we ever need it again.
- templated_hanger_path = user_closet_hanger_path(user_id: '$0', id: '$1').sub('$0', '${user_id}').sub('$1', '${closet_hanger_id}')
%script#closet-hanger-update-tmpl{type: 'text/x-jquery-tmpl'}
= form_tag templated_hanger_path, method: :put, authenticity_token: false, class: 'closet-hanger-update' do
= hidden_field_tag 'closet_hanger[list_id]', '${list_id}'
= hidden_field_tag 'closet_hanger[owned]', '${owned}'
= number_field_tag 'closet_hanger[quantity]', '${quantity}',
min: 0, required: true
%script#closet-hanger-destroy-tmpl{type: 'text/x-jquery-tmpl'}
-# TODO: remove me?
%script#closet-hanger-destroy-tmpl{type: 'text/x-jquery-tmpl'}
-# TODO: remove me?
- content_for :stylesheets do
= stylesheet_link_tag 'https://ajax.googleapis.com/ajax/libs/jqueryui/1.9.0/themes/south-street/jquery-ui.css'
@ -154,4 +153,15 @@
- content_for :javascripts do
= include_javascript_libraries :jquery, :jquery_tmpl
= javascript_include_tag 'ajax_auth', 'lib/jquery.ui', 'lib/jquery.jgrowl',
'lib/stickUp.min', 'closet_hangers/index'
'closet_hangers/index'
- if public_perspective? && user_signed_in?
- content_for :meta do
%meta{
name: "trade-matches-owns",
value: @items.select(&:owned?).map(&:id).join(",")
}
%meta{
name: "trade-matches-wants",
value: @items.select(&:wanted?).map(&:id).join(",")
}

View file

@ -18,13 +18,15 @@
- if show_controls
= render partial: 'closet_lists/trading_neomail_warning', locals: {list: closet_list, user: @user}
.closet-list-content
- if closet_list.description?
= closet_list_description_format closet_list
-# Cachebuster comment: Updated downstream content at Feb 20 2024, 6:15pm
- cache closet_list do
.closet-list-content
- if closet_list.description?
= closet_list_description_format closet_list
.closet-list-hangers
- unless closet_list.hangers.empty?
= render_sorted_hangers(closet_list)
%span.empty-list= t('.empty')
.closet-list-hangers
- unless closet_list.hangers.empty?
= render_sorted_hangers(closet_list)
%span.empty-list= t('.empty')

View file

@ -1,4 +1,6 @@
-# NOTE: Changing this requires bumping the cache at `_closet_list.html.haml`!
= link_to item_path(item) do
= image_tag item.thumbnail_url, :alt => item.description, :title => item.description
= image_tag item.thumbnail_url, alt: "Thumbnail for #{item.name}",
title: item.description, loading: "lazy"
%span.name= item.name
= nc_icon_for(item)