Thorvarium / vine-styling

Styles to use with vine
https://discord.gg/C3xDJTTGhh
MIT License
22 stars 13 forks source link

Suggestion for 'details' popup window #20

Open robertlipe opened 3 months ago

robertlipe commented 3 months ago

It's beyond belief that Amazon's web team hardcodes pixel widths here in 2020.

I've resisted the urge to show a 'before' picture for confidentiality but can do so if reached at the obvious gmail address. On my 27" ("27 inches, Bob!") monitor in Chrome, the 'show details' box turns ugly if I give the vine window any less than half my screen. Now that I've dug unto the CSS, I understand why the popups for item details are unreadable on mobile. :-(

As a developer myself, I know that what you really want is a PR that you can just click 'merge and close', but I also know that you're likely to debate how and where to best integrate this and that since this is essentially a one-liner, I hope you can easily copy-and-paste this into the official release without such back-and-forth. (Which I will do if that's really needed.)

The simple addition of

.a-popover { width:90%; }

to the bottom of my stylebot makes "See details" actually look reasonable for any width that I can resize my browser to on a 27" display. Notably, the "narrow" sizes don't simply break and if I could do this on mobile chrome, I wouldn't have to the screen sideways to read the 'Details' pop-up (Hard to believe their developers never tested on a mobile device. Sigh).

OK, I started with a one-liner, but then I looked carefully at the rest of that dialogue. This disease runs deep, but I lack the CSS mojo to "fix" it. Their developers were completely too clever with jquery (OMG, it's 2010!) instead of just making a stinking .flex layout and letting that right hand column grow-and-flow.

id="vvp-product-details-info-container" Is the right hand column in a flex that's effectively the image on the left and the name, vendor, and (artificially truncated) description. It's insane on a modern screen to limit that to an expander of 150px and force a 'more' but that's where we are. It also holds the drop-downs for any options like color or size.

We would like a-popover-inner to be able to use all that massive whitespace that's before back/request buttons, but from the factional pixel notation, we assume that Javacript is strategically lying to the layout engine about the wanted height of that div.

vvp-product-description-expander seems to be the center of the storm. It's artifically capped at 150px (ugh, more hard-coded nonsense) and relies on Javascript (aui-da-a-expander-toggle?) to knock it back to data-a-expander-collapsed-height when minimized. Change either of the and the layout falls over. I didn't chase it up into the (surely obfuscated) Javascript.

It seems that the ...expander relies on ...expander-toggle() to resize vvp-product-description-expander and thus, vvp-product-details-info-container to include the expansion PLUS vvp-product-details-modal--variations-container (which it seems to forget - or at least lose) and it's thus hidden and worse, without a scrollbar which we can fix with an overflow. I'd like to lose the paginator and just rely on scrolling when needed. The descriptions are rarely THAT long, thought mobile might pick other chocies. (That are still > 150px on a modern screen.)

So I guess please accept my one line change and I'll just write off a few hours of programmer loss trying to make it look sensible. It's goofy looking, but it's at least readable now, so it's hard to take that as total defeat.

It's, of course, your choice whether to close issue when when the original fix is applied or to keep it open and pursue aliminating thta paginator together.

Thank you for making this and keeping it awesome!

Thorvarium commented 3 months ago

I don't understand the issue sorry. A screenshot might help. I also have a 27 inch monitor

robertlipe commented 3 months ago

Sorry. I can ramble.

The narrow window case is the troublesome one. It also makes it super frustrating on mobile, where you often have to rotate the popup to even read it.

Here's an example. Notice the buttons that are off the right edge of the screen and the text that's clipped. This should be enough for you to recognize the window without breaching our NDAs.

With my 'width' fix, it stays on the screen no matter how narrow the Chrome window is.

Thanx for listening.

robertlipe commented 3 months ago
Screenshot 2024-07-01 at 10 27 08 AM
Thorvarium commented 3 months ago

@robertlipe mobile has special styles designed for it. You can't use the desktop ones hehe. You can find android and ios variations here on the repo as well.

Are you using on a third of the screen or something on desktop? For me it shows fully even when using half monitor

robertlipe commented 3 months ago

Agreed. Mobile is a different problem to solve.. I was just offering that as an example of a screen that looks consistently terrible, even without your awesome mods.

On my desktop, it's admittedly a more minor problem. On my 13" MBP, if the window is much less than the full screen, the hard coded 900px width breaks it in many cases. This window is just a problem in many of my screens.

On Mon, Jul 1, 2024, 12:23 PM Thorvarium @.***> wrote:

@robertlipe https://github.com/robertlipe mobile has special styles designed for it. You can't use the desktop ones hehe. You can find android and ios variations here on the repo as well.

Are you using on a third of the screen or something on desktop? For me it shows fully even when using half monitor

— Reply to this email directly, view it on GitHub https://github.com/Thorvarium/vine-styling/issues/20#issuecomment-2200669170, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD34X7BNTT632ITEKXA3ZKGGADAVCNFSM6AAAAABKDXGFGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBQGY3DSMJXGA . You are receiving this because you were mentioned.Message ID: @.***>

robertlipe commented 3 months ago

Harder to Greek these as I'm on mobile. This is what it looks like o desktop, too, but there's less that we can do about that. It looks less terrible on desktop, but with my fix, the second screen would be usable at least.

Screenshot_20240701-143417 Screenshot_20240701-143408

I hope I'm not making things more confusing by continuing to bring up mobile...