Skip to content
This repository has been archived by the owner on May 22, 2020. It is now read-only.

Fix css centering on mobile #268

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gusano
Copy link

@gusano gusano commented Sep 2, 2017

On some mobiles the modal is not centered (see screenshots below).
This fixes it by using flexbox.

Before

deepinscreenshot_select-area_20170902211437
deepinscreenshot_select-area_20170902211452

After

deepinscreenshot_select-area_20170902211525
deepinscreenshot_select-area_20170902211540

@gusano
Copy link
Author

gusano commented Sep 2, 2017

I don't know how minifying css is done (didn't use Grunt in a looong time) but it looks like dist/ekko-lightbox.min.css hasn't changed since a year ago, otherwise I would happily add the change there as well.
Meanwhile I'm gonna use my fork :)

Thanks for the nice lib!

@gusano gusano changed the title Fix css centering Fix css centering on mobile Sep 2, 2017
@ashleydw
Copy link
Owner

ashleydw commented Sep 3, 2017

As this plugin uses bootstraps modals, isn't this a problem in bootstrap rather than the plugin?

@VirginieGarcin
Copy link

Indeed it's the default behaviour of bootstrap modal.
It's not only on mobile: bootstrap modal is always shown at the top of the page. (On desktop as the display is landscape we will not notice it for classic pictures, except for panoramic images.)

Even if it's the default modal behaviour, the result would be nicer if the image was centred vertically :)

Copy link
Contributor

@RafaPolit RafaPolit left a comment

Choose a reason for hiding this comment

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

There is an un-closed bracket on the CSS file, probably on line 12.

If you meant to include everything inside the container, then the indentation and bracketing is still wrong.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants