kakaroto / Beyond20

D&D Beyond Character Sheet Integration in Roll20
GNU General Public License v3.0
497 stars 145 forks source link

proposed fix for width in beyond20 settings #939

Closed jjchambl closed 2 years ago

jjchambl commented 2 years ago

Set a max-width property to the ajs-dialog of 1250px; this was tested on an ultrawide 1080p monitor and a 16:9 monitor. Feel free to test and adjust the pixel value set in the beyond20.css file at the bottom. I've commented the relevant section.

Aeristoka commented 2 years ago

image

This appears not to have worked, as something is overriding it to max-width: none;

Aeristoka commented 2 years ago

Appears to be working now, as discussed on Discord (the important !Important flag).

kakaroto commented 2 years ago

Humm... I think I would have left the alertify size as is but instead added the max-width to the div that we display inside of it. Not sure if it would look good or not though.

jjchambl commented 2 years ago

I can certainly try that, but the issue is two-fold: 1) alertify includes the !important flag on that combination of classes so we would need to include that regardless, 2) it seems that alertify is overriding bootstrap's own styles, in which case the .dialog class marker is the surest way to affect the sizing of the modal, otherwise the entire modal will still take up the entire width of the screen, 3) i just now saw where we're adding classes in javascript lol. Let me know what you think and i can make changes as needed.

kakaroto commented 2 years ago

I can certainly try that, but the issue is two-fold: 1) alertify includes the !important flag on that combination of classes so we would need to include that regardless, 2) it seems that alertify is overriding bootstrap's own styles, in which case the .dialog class marker is the surest way to affect the sizing of the modal, otherwise the entire modal will still take up the entire width of the screen, 3) i just now saw where we're adding classes in javascript lol. Let me know what you think and i can make changes as needed.

I gave it a try and I don't like it 😬 image (the change was simply to add max-width: 900px; margin-left: auto; margin-right: auto; to the existing min-width style of <body> in options.html)

Your fix looks much better and cleaner, so I'm merging it, thanks!