Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Monster Frame background in Safari for iphone and ipad #3526

Merged

Conversation

5e-Cleric
Copy link
Member

Safari for mobile screens does not support background-blend-mode and background-repeat:repeat; in the same element, it deactivates the overlay, which makes our statblocks look grey:

image

A fix is set here, i manually overlayed the color with the image to make the background, and i'm changing it for the original.

This fix works, and other users will not notice the difference.

@5e-Cleric 5e-Cleric added tweak Small, non-breaking change ❌ Missing from V3 This should be in v3 🔍 R0 - Needs first review 👀 PR ready but has not been reviewed labels Jun 8, 2024
@5e-Cleric 5e-Cleric self-assigned this Jun 8, 2024
@calculuschild
Copy link
Member

calculuschild commented Jun 8, 2024

other users will not notice the difference.

The use of a grayscale image and blending was a deliberate choice for customizability of the monster stat block color (similar to what @Gazook89 was proposing for the whole page background in #3218). Changing this will break brews that have used this to adjust the color of the monster stat block.

@5e-Cleric 5e-Cleric temporarily deployed to homebrewery-pr-3526 June 8, 2024 22:43 Inactive
@5e-Cleric 5e-Cleric temporarily deployed to homebrewery-pr-3526 June 10, 2024 20:09 Inactive
@dbolack-ab
Copy link
Collaborator

Is this a can't fix at Safari?

@5e-Cleric
Copy link
Member Author

5e-Cleric commented Jun 12, 2024

Is this a can't fix at Safari?

https://bugs.webkit.org/show_bug.cgi?id=259081

There is the reported bug in webkit.

@5e-Cleric 5e-Cleric added the P2 - minor feature or not urgent Minor bugs or less-popular features label Jul 25, 2024
@calculuschild
Copy link
Member

Can someone with an iphone confirm the fix works (screenshot?) and also see what happens with this brew where the monster block is blended to be green?

https://homebrewery.naturalcrit.com/share/_h7ThoQlrJVc

@Gazook89
Copy link
Collaborator

Gazook89 commented Aug 1, 2024

iPhone 17.5.1

image

@Gazook89
Copy link
Collaborator

Gazook89 commented Aug 1, 2024

Wait hang on

@Gazook89
Copy link
Collaborator

Gazook89 commented Aug 1, 2024

Sorry I just opened the non deployment link without thinking, that’s the previous screenshot.

Here is from the PR deployment, but without the green bit because it’s pretty difficult to open the css editor on my tiny screen (iPhone 12 mini):

image

Note, the top and bottom bars are cut off but a different issue

@5e-Cleric
Copy link
Member Author

The border image is not slicing as it should, but that doesn't surprise me, i did not provoke it, and it is out of the scope of this PR.

@calculuschild
Copy link
Member

calculuschild commented Aug 1, 2024

it’s pretty difficult to open the css editor on my tiny screen (iPhone 12 mini)

How about this link?

https://homebrewery-pr-3526.herokuapp.com/share/_h7ThoQlrJVc

I can confirm that in chrome, green monster block still works in this deployment. 👍
image

@@ -7,6 +7,7 @@
@noteBorderImage : url('/assets/noteBorder.png');
@descriptiveBoxImage : url('/assets/descriptiveBorder.png');
@monsterBlockBackground : url('/assets/parchmentBackgroundGrayscale.jpg');
@monsterBlockOverlay : url('/assets/parchmentBackgroundOverlayed.jpg');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This graphic isn't used anymore, right?

@calculuschild calculuschild added 🔍 R3 - Reviewed - Awaiting Fixes 🔧 PR is okayed but needs fixes before merging and removed 🔍 R0 - Needs first review 👀 PR ready but has not been reviewed labels Aug 1, 2024
@calculuschild calculuschild temporarily deployed to homebrewery-pr-3526 August 1, 2024 21:07 Inactive
@Gazook89
Copy link
Collaborator

Gazook89 commented Aug 1, 2024

Thanks for the link, here it is:

image

@calculuschild
Copy link
Member

Ok, this seems to be working without messing up custom coloring. Thanks @5e-Cleric ! And thanks @Gazook89 for testing. Will merge now.

@calculuschild calculuschild merged commit bb06a3e into naturalcrit:master Aug 1, 2024
2 checks passed
@5e-Cleric 5e-Cleric deleted the fix-background-monster-in-safari branch August 1, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❌ Missing from V3 This should be in v3 P2 - minor feature or not urgent Minor bugs or less-popular features 🔍 R3 - Reviewed - Awaiting Fixes 🔧 PR is okayed but needs fixes before merging tweak Small, non-breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants