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

DS-699 Safari Image Position Bug #2422

Merged
merged 9 commits into from
Jan 25, 2022

Conversation

MarcinMr
Copy link
Collaborator

Jira

https://pegadigitalit.atlassian.net/browse/DS-699

Summary

An image center position on Safari was fixed

Details

When max-width: value; inline style is applied to an <img> and modal width property is set to auto, the image should stay in the center position.

How to test

Pull the branch. Add an inline style to an image for example max-width: 600px and set the modal width to auto, Check if modal width adjusts to the image width and the image stays in the center position.

@github-actions github-actions bot added the type: bugfix List this PR in the 'Bug Fixes' section of the release notes. label Jan 21, 2022
@colbytcook colbytcook requested a deployment to feature/DS-692-animation-tool-set--12e9d500--commit-preview January 21, 2022 11:24 In progress
@MarcinMr
Copy link
Collaborator Author

@mikemai2awesome

Here https://boltdesignsystem.com/pattern-lab/?p=components-modal-trigger-variations is written that:

"Advanced usage: if the Image component has an absolute value (e.g. 640px) defined for max_width, then the modal's width prop can be set to auto. This will allow the image inside the modal to be responsive but does not stretch beyond the specified max_width."

But this didn't work well and the image wasn't responsive below its max-width declaration, like in the video example:

image.modal.unresponsive.mov

I added the width: 100% to the image that exists inside a modal, but wasn't sure if it should be declared in the modal.scss like now or they should add the width property using inline style every time the max-width is set.

Just curious, do we have/use user-agent classes depending on what browser the user has? Wouldn't it be good to fix bugs related to the browser? Because sometimes fixing something on one browser can break things on another.

Copy link
Collaborator

@mikemai2awesome mikemai2awesome left a comment

Choose a reason for hiding this comment

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

@MarcinMr I reverted some of your CSS, the docs are outdated. All images must have width and height attributes defined, this will ensure they display correctly.

I also fixed some of the docs to be more clear.

@colbytcook colbytcook had a problem deploying to bugfix/DS-699-Safar-Image-Position-Bug--branch-preview January 21, 2022 22:10 Failure
@colbytcook colbytcook temporarily deployed to bugfix/DS-699-Safar-Image-Position-Bug--bc3f3ec8--commit-preview January 21, 2022 22:42 Inactive
@colbytcook colbytcook had a problem deploying to bugfix/DS-699-Safar-Image-Position-Bug--branch-preview January 21, 2022 22:54 Failure
@colbytcook colbytcook had a problem deploying to bugfix/DS-699-Safar-Image-Position-Bug--branch-preview January 24, 2022 17:50 Failure
@colbytcook colbytcook had a problem deploying to bugfix/DS-699-Safar-Image-Position-Bug--branch-preview January 25, 2022 15:37 Failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bugfix List this PR in the 'Bug Fixes' section of the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants