There are a couple spots where we parse SWF URLs to get the ID out! Most visibly, our Support tools were crashing on it. And internally, manifest loading wasn't working. (I'm not sure if this got caught or if it caused crashes in user space? I didn't see them when wearing a failing item)
Anyway, fixed now!
This is because it's the terminology I'm using elsewhere ("Items" and "Closet" are too overloaded in the UI), and because I want to start putting specific lists at like `/user/:userId/lists/:listId`!
I also create a redirect from the old URL, and also from the DTI Classic variant of the URL
Ah right, React state batching doesn't always work how I expect it to. The separate state caused the hook to return and cache `{loading: false, error: null, data: null}`, and then on a _later_ tick the data value showed up, but only _after_ the response was already cached!
This broken a bunch of species/color picker stuff, now it's fixed!
Okay, so getting the initial render down time for these faces is annoying, though I might come back to it…
But actually, the _worst_ part isn't the _initial_ render, which just kinda gets processed as part of the page navigation, right?
The _worst_ part is that we render it slowly _twice_: once on page load, as we send the `useAllValidPetPoses` fetch request; and then again when the fetch request ~instantly comes back from the network cache.
The fact that this requires a double-render, instead of just rendering with the cached valids data in the first place (like how our GraphQL client does), causes a second and highly-visible render of a slow-to-render UI!
So, here we update `useAllValidPetPoses` to cache its response in JS memory, similar in principle to how Apollo Client does. That way, we can return the valids instantly on the first render, if you already loaded them from the homepage or the wardrobe page or another item page!
This is a pretty easy change, that makes re-renders faster when something about the item preview state changes!
That said, the initial render is still pretty slow, too, and that's the one that's bothering me more lol
Ah oops, because I forgot to set `hiResMode` here in the image preloader, we would preload the PNG, and _then_ load the SVG separately.
This doubled the effective image loading time in Hi-Res Mode!
Now, the image preloader respects hi-res mode, and will preload the SVG in the SVG case, and the PNG in the PNG case.
If you're not in hi-res mode, then you don't care about broken SVGs, because you wouldn't have seen them anyway!
We also update the message to reference Hi-Res Mode.
We're just having too many glitchy SVGs for my taste, esp since TNT seems to just be using PNGs for now?
This change defaults us to using PNGs for users by default, with the option to use SVGs as a new "hi-res mode" setting.
This is our first ever setting, wow!
I'm also envisioning that like, if we get Fastly Image Optimizer set up, this could be a way to tune the quality of the incoming images.
We could also consider a setting to turn off animations altogether—like, just download the PNG instead of the movie, whereas right now we download the movie on the assumption that you might play it at any time.
The way we were checking for UC compatibility issues, was also triggering while the appearance was still loading, so items didn't have any appearance layers yet!
Now, we check for loading before testing for that glitch.
Oh right, adding user data to this query makes it uncacheable!
Split the query into the main public data, which will cache; and the user data, which will load in later.
I refactor the hide-badge thing into 3 "trade matching modes". Then, the logic for whether to hide specific badges moves into the component, and we use that same flag to decide whether to show the big word "match"!
Boom, cute owns/wants badges on "Latest items", and the item search page, and trade matches!
I'm gonna add some additional flair to the trade match case, too!
This has been bugging me for a while lol, the background was leaking out of the corners!
I had applied the styles to the `InputGroup` because I didn't realize how Chakra implements this… I had assumed that the left/right elements wouldn't also get the background.
But it turns out, `InputGroup` uses `position: absolute` stuff, and uses padding to create visual space in the `Input` below them! So, this works perfect!
Copied styles from the similar layout in the bulk converter tool! The status will flush to the right of the field header on desktop, and move below the input on mobile.
Mm right, when we first render the output, `imageUrl` is `undefined`, so the output textbox renders with `value={undefined}`, which is an "uncontrolled" component that the DOM is free to change.
In practice, this isn't an issue because the textbox has `isReadOnly`, so the user can't _actually_ change it. But it's still a good idea for consistency and clarity to use an empty string instead of `undefined`, and it removes warning spam from my console!
Oops, previously the MajorErrorMessage was willing to shrink the width of the cute Grundo Programmer icon, to allow error messages with long words to avoid word breaks.
Here, we switch `1fr` for `minmax(0, 1fr)`, which allows the text zone to get smaller. (`1fr` is short for `minmax(auto, 1fr)`, which isn't capable of shrinking smaller than the natural value.)
Now, the error text is more willing to shrink by word-wrapping, than the image is by shrinking the image. Success!
Oops, my cute API idea for `speciesPickerProps` breaks `React.memo`, of course!
We could fix this by having the caller memoize the `speciesPickerProps` object, but that's too unusual and error-prone. We could also fix this by writing a custom function for `React.memo` to determine whether props match, but that seems like overkill when there's only one actual prop we're using here in practice.
So yeah, I've updated `SpeciesColorPicker` to just accept `speciesTestId` and `colorTestId` props instead!
Note that actually this component _is_ still re-rendering too often, because of a Chakra bug I just discovered and reported! So this change won't immediately improve performance, but it should stop re-rendering too often once we _also_ upgrade Chakra after this bug is fixed. https://github.com/chakra-ui/chakra-ui/issues/4080
This folder will include code shared by both the client-side app and the server!
The server isn't using it yet, but it will in a new API endpoint soon! I'm doing this in a separate commit to avoid lumping all the import-change noise into that commit.
I'm doing this in preparation for an API endpoint to build outfit images by ID. It'll need the same logic to decide which layers are visible, and the same GQL fragments to load the relevant data!
Oh right, labeling PB items as NP is confusing! Here, we add a "PB" case to the lil badge on the corner of the item thumbnail, in item search page & homepage Newest Items.
Oops, we did an in-place sort on the search variables we passed to Apollo! This meant that Apollo's first read of the variables wouldn't match later reads, so it would always decide the variables had changed, causing an infinite re-render loop.
Remember to copy existing arrays before sorting! 😅
Incidentally, this only happened for Markings, by coincidence: it's the only (I think) searchable zone label with multiple zone IDs, that don't sort alphabetically the same as they sort numerically. This `.sort()` sorts them alphabetically, whereas they come in numerical order in `allZones`, because that's the order the GQL server returns them in `build-cached-data.js`.
There have been usability problems with this search filter UI, and I think they mostly come down to people accidentally selecting filters when they don't mean to—sometimes pressing Enter to indicate that they're done typing, but accidentally selecting something.
Here, we remove that behavior, and additionally add a new behavior to clear the suggestions on pressing Enter.
Boom, pulled some stuff out into util! Now we also have error boundaries in both panels of the wardrobe page.
You can test this one by visiting `/outfits/new?send-test-error-for-sentry`, just like on the home page! Util component for fake errors owo
Previously, we would tear down to a blank white page. Now, for errors within most page content, we show a cute error message with a grundo programmer!
To test, visit `/?send-test-error-for-sentry`, which will trigger an intentional render error on the home page.
Note that this does _not_ cover pages that don't use PageLayout, namely the wardrobe page! I'll want to add other boundaries there…
Crashes on clearing the search box. I really thought I fixed this when I refactored `forceReset` to be a function, but I guess I missed it when I went back-and-forth deciding whether to actually do the refactor!
Before this, if you made a change while the outfit was auto-saving, it would reset your changes back and forth in an infinite loop, oops!
This was because the response from the save would reset the outfit state to match, but the _debounced_ outfit state would still show the user's changes, so we'd trigger another save. And then the same thing would happen in reverse, and back and forth again!
Oops, it was possible after saving an outfit to get into a state where we would show the `<OutfitThumbnailIfCached />` behind the outfit even after it was saved, and then removing items would look weird until auto-saving caught up.
We had used the `backdrop` property because we wanted smoother partial load-ins, but for now I'm just fixing this by switching it to `placeholder`, which already has the right loading-only behavior.
This was also the only call site for `backdrop`, so I've removed it!
Oops, the sequence here was:
1) Save a new outfit
2) The debounced outfit state still contains id=null, which doesn't match the saved outfit, which triggers an auto-save
3) And now again, the debounced outfit state contains the _previous_ saved outfit ID, but the saved outfit has a _new_ ID, so we save the _previous_ outfit again
and back and forth forever.
Right, ok, simple change: if the saved outfit ID changes, reset the debounced state immediately, so it can't even be out of sync in the first place! (I also considered checking it in the condition, but I didn't really understand what the timing properties of being out of sync due to debouncing would be, and it seemed to not represent the reality I want.)
Hope it actually work-works lol
Did some refactors in useOutfitState to support the new reset action we do after auto-saving, in case the server tweaked things like the name.
Note that we implemented the actual horn behavior described in the message, simply by marking the yellow horn appearance glitched for Fem, but not for Masc! Also, we don't have a yellow-horn Sick Masc model, so it's blue too.
It wouldn't open, because I'd set `isLazy` on the popover, so opening the modal would close and UNMOUNT the popover, which unmounted the modal!
Now, we use the new `lazyBehavior` prop to keep it mounted _after_ the first time it opens. This is why I needed to upgrade Chakra!
Oops, I made a recent change to automatically add `appearanceId` to the outfit state when you open the Support pose picker, to avoid navigation issues.
But I didn't realize this happened _silently_ when you open the page as a Support user, because the Popover preloads!
Now, the Popover doesn't preload its content. This is probably better for normal users too, the PosePicker UI is a bit heavier with 6 previews than I really want!
Oops, our "items to reconsider" feature was preventing unwearing/removing items you're already wearing!
This feature helps you try stuff in Search, without disrupting your outfit. e.g. if you try on a new Background, then change your mind and unwear it, then we reapply whatever old Background you had on the outfit before.
But this made it impossible to remove your _current_ background from the search page if you went back and searched for it again, because we would remove it and then reconsider and reapply it 😅
Now we, um, stop that!
Huh, dunno when I regressed this! Or maybe I never did it for search results, just the main items page? But we're needlessly re-rendering the entire search results list when you wear/unwear something, because `onRemove` always changes, and that breaks the `React.useMemo` on `Item`.
Now, we cache the `onRemove` callback with `React.useCallback`, so perf is much happier!
Oops, we extracted Support fields out from the default `appearanceLayerFragment`!
This was causing the page to silently fail to show any changes, because `layer.remoteId` was evaluating to `undefined` rather than one of the ID numbers in the range.
Here, I've added both `remoteId` explictly because we use it directly, and also the support fields because that's what the layer support UI needs!
Oops, making changes in PosePickerSupport would sometimes trigger a re-fetch in PosePicker.
Specifically, PosePicker needs some fields that PosePickerSupport doesn't, so changing the canonical poses causes PosePicker to ask for stuff again—which will probably serve a SWR'd cached version that doesn't reflect the Support changes!
Here, we update the PosePickerSupport query to prefetch all the fields the PosePicker _would_ want for any of these poses. That way, if we swap in a new one as the canonical appearance for a pose, there's no refetch needed, and therefore no risk of hitting a stale cache.
We move to an actual GQL query, instead of approximating with /api/validPetPoses.
Notable changes are omitting glitched states from UNKNOWN, so we don't prompt Support users to fill in missing states with bad states; and omitting glitched states from standard, so that we _do_ prompt Support users to check UNKNOWN states for new _non-glitched_ versions we can start to use.
Now, when viewing a saved outfit that you own, you'll see a "Saved" indicator if it matches the version on the server, or a temporary UI of "Not saved" and a tooltip if not.
Auto-save coming next!
Previously, the PNG link for a pet layer would show the 150x150 version. This was both an inconvenient size, but also not reflective of how the layer actually behaved, because we only use Neopets's official PNG for the 600x600 version!
Ah, oops, the `id` field from `useOutfitState` went missing and I didn't notice, so `useOutfitSaving` didn't correctly detect that this was an existing outfit!
This made saves on existing outfits create new copies, which isn't a bad behavior exactly, but I don't want to go there; saving a copy is just gonna pollute people's outfit lists rn, worse than no option imo.
Just a basic e2e starting point! Simple logic, with simple gates to prevent saving outfits we're not ready for. Safe to ship, despite being very incomplete!
The layer preloader already takes advantage of, and primes, the HTTP cache.
But we still do duplicate work, on every OutfitPreview render, to re-execute movie clip libraries, and create a movie clip to test for animations. The former is nontrivial cost, and the latter is often large cost. This can make even basic outfit changes slow, when there's no change to the movie clip layers and the player is paused!
Here, we add an LRU cache for movie clip libraries, and for the question of "is it animated?". This should speed up a number of places where we would reload the movie (including between toggling the item), and various changes that were triggering full movie clip rebuilds unnecessarily.
We _aren't_ solving here for the fact that toggling an animated item requires rebuilding the movie clip, which could conceivably be cached—but with some state management trickiness, because ideally it should be a separate clip for each context where it's being shown. Imo not yet worth the effort! (esp because I think users understand that toggling an animated item can be slow, whereas this was affecting _other_ actions way too much)
Here, we consolidate useOutfitState to get its base `outfitState` from a few different places: parsing the URL, and processing the saved outfit data, return an object of the same shape as the state stored in the reducer.
This enables us to just pick one of the three, instead of our kinda awkward individual-field fallbacks.
This will also help us with some upcoming work to make Back/Forward navigation work better.
Now we show a message for pet layers with the DISPLAYS_INCORRECTLY_BUT_CAUSE_UNKNOWN glitch applied! Previously, there would just be no message, because we only had UI logic for when it was applied to an _item_ layer.
Some of the "MiniMME11-S1: Approaching Eventide Skirt" visuals are pretty clearly glitched on TNT's end, like the Jubjub, which just has a single flat version of the dress floating in the corner of the screen.
This is a message to make that case even clearer!
I'm applying this to the "MiniMME11-S1: Approaching Eventide Skirt" on the Acara, which seems to load all 1000 images from the manifest, but then show no animation and no errors. Not sure what's up, and not inclined to deep-debug until we have a check on whether it works on-site!
I wanted the ability to clear out closet list text for Support users, and figured I should just build the UI for end users too, and grant Support users the same access!
This helps with items like "Living in Watermelon Foreground and Background", which has a species-specific foreground and bodyId=0 background.
With this flag set on the background, it won't appear for pets that don't _also_ have something else that fits. In this case, it hides it from Standard Vandas, and all non-standard colors.
There's some hacky limitations here: the item page still highlights the Vanda, even though clicking gives nothing; and the zone info for it is messy too, with the Background claiming to fit all species, and the LFI claiming to fit 54 specific species. But those don't seem important enough to code for!
I like making these more concise and consistent across the dropdown and the cards, I'm still not 100% sold on the design but it seems ok for now!
At first I had it in the dropdown as "Background (3)", but realized that conflicts with the usual pattern of like, saying how many items match a certain filter…
Using this to get an at-a-glance check on how Neopets IDs are typically assigned for body-specific items… looks like it's increasing, and alphabetical by species? (not by species ID?)
I think this will be generally useful to minimize switching around for common operations, but also I'm thinking of building a bulk assign tool for things with broken body IDs, and this will be the place for it to live, I think
Oops, the HTML5Badge was using the presence of an `svgUrl` to decide if the item is converted!
Here, we add an extra condition that if the OFFICIAL_SVG_IS_INCORRECT glitch is applied, then *that* indicates HTML5 conversion, too.
