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

Fixed logo scaling #2

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

Fixed logo scaling #2

wants to merge 3 commits into from

Conversation

regebro
Copy link

@regebro regebro commented Jul 8, 2013

The logo would only scale on Chrome, and then it would scale wrongly if the viewport was big. I fixed this so it will be full size when the viewport > 610px and scale nicely when it's < 610px. Tested on Chromium 28, FF 22 and IE 10.

@RobZoneNet
Copy link
Member

Lennart,

Hello how are you?

Line 34 of this file https://github.com/regebro/collective.responsivetheme/blob/2b6f4fd96e85a0d9776eaac4ad65d52b3eb9d68e/collective/responsivetheme/skins/collective_responsivetheme_styles/mediaqueries.css is causing the issue(300% width). I put all the mediaqueries in one file.

I see you took out the width out of logo.pt too.

Anyways I wouldn't mind correcting the mediaqueries.css. and not changing responsivetheme.css. Also for responsive it should be max-width: 100%; not width:100%; (only width:100% for older IE's). I could be ok with taking out the width out of logo.pt if you think that is hurting something.

What are your thoughts?

Rob

On Jul 8, 2013, at 3:00 AM, Lennart Regebro notifications@github.com wrote:

The logo would only scale on Chrome, and then it would scale wrongly if the viewport was big. I fixed this so it will be full size when the viewport > 610px and scale nicely when it's < 610px. Tested on Chromium 28, FF 22 and IE 10.

You can merge this Pull Request by running

git pull https://github.com/regebro/collective.responsivetheme master
Or view, comment on, or merge it at:

#2

Commit Summary

Fixed logo scaling.
File Changes

M collective/responsivetheme/skins/collective_responsivetheme_styles/responsivetheme.css (9)
M collective/responsivetheme/templates/plone.app.layout.viewlets.logo.pt (2)
Patch Links:

https://github.com/RobZoneNet/collective.responsivetheme/pull/2.patch
https://github.com/RobZoneNet/collective.responsivetheme/pull/2.diff

@regebro
Copy link
Author

regebro commented Jul 9, 2013

I mainly made a pull request because there was no issue tracker. :-) You fix the issue as you think is best.

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

Successfully merging this pull request may close these issues.

None yet

2 participants