exile-center / better-trading

QoL improvements for the official PathOfExile trading site.
30 stars 5 forks source link

Fix pinning #109

Closed dbjorge closed 1 year ago

dbjorge commented 1 year ago

Based on Chrome store reviews, it looks like the pinnable enhancer broke sometime in May. It looks like the issue was that the container element that the enhancer attempts to inject its button into changed the name of a style we used to query for it (from .details .pull-left to .details .btns).

This PR adjusts the selector to put our Pin button in the expected place.

It looked like the styling was a bit off from the other nearby buttons - presumably the source page styling we were matching changed slightly alongside the change that broke us. I adjusted the styling such that our button reuses the same btn and btn-default classes used by the other sibling buttons and uses a wrapping <span> element similar to the other buttons to match their margin behavior, and then eliminated our slightly-broken copies of those styles.

Finally, to avoid an issue where the button's width changes between the "Pin" and "Unpin" states (since the text has different length) and the layout shifts around at certain breakpoints when toggling states because of the width difference, I added a min-width corresponding to the "Unpin" state so that the button remains the same size between states. This is a best-effort thing - it might not fix the layout shifts for users that use fonts besides the standard english Windows+Chrome one I'm testing against, but it should slightly improve the common case and it's not that bad for the cases where it doesn't help.

The flexbox behavior with the other buttons feels a little janky because the existing buttons are a little janky; the buttons are contained in a flexbox with space-between, but the "direct whisper" button with its dropdown has margin-right: auto, so the buttons have inconsistent spacing behavior already that becomes even weirder feeling with our new button. But it works reasonably well at what I expect to be the most common use scenario (compact layout at browser width > ~1280px) so I don't feel inclined to mess with it too much.

Before (missing button): screenshot highlighting button group with no pin button

After (button exists and works as expected): animation demonstrating pin button behavior in different sizes and layouts