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

Media queries for reveal #4780

Closed
pepebe opened this issue Mar 24, 2014 · 5 comments
Closed

Media queries for reveal #4780

pepebe opened this issue Mar 24, 2014 · 5 comments
Assignees
Milestone

Comments

@pepebe
Copy link

pepebe commented Mar 24, 2014

Just noticed that lines 3454+ in foundations.css inv.5.2.1 look fishy...

Identical min-width rules in each mediaquery (min vs. min) as well as the same rules for several of them in a row.

I don't have the time right now to do more research and before I forget to do anything, I think its better to at least post this here.

Regards,

pepebe

  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog, .reveal-modal {
      top: 6.25rem; } }
  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog.tiny, .reveal-modal.tiny {
      margin-left: -15%;
      width: 30%; } }
  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog.small, .reveal-modal.small {
      margin-left: -20%;
      width: 40%; } }
  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog.medium, .reveal-modal.medium {
      margin-left: -30%;
      width: 60%; } }
  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog.large, .reveal-modal.large {
      margin-left: -35%;
      width: 70%; } }
  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog.xlarge, .reveal-modal.xlarge {
      margin-left: -47.5%;
      width: 95%; } }

  @media only screen and (min-width: 40.063em) and (min-width: 40.063em) {
    dialog.full, .reveal-modal.full {
      margin-left: -50vw;
      width: 100vw; } }

@vinayraghu
Copy link
Contributor

@pepebe Thanks for bringing this up.

This is a well known issue with sass. sass/sass#316 and sass/sass#116

Currently using media queries through variables in sass will lead to multiple declarations of the same media query. Ideally you would want all of the media rules to be clustered into a single query.

However, if the files are minified and gzipped, this might be a non-issue as gzipping identifies recurring patterns and compresses them.

If you're still interested, you might want to check out https://github.com/aaronjensen/sass-media_query_combiner

@vinayraghu
Copy link
Contributor

Apologies, I just noticed the multiple min widths...Will take a look into this

@pepebe
Copy link
Author

pepebe commented Mar 24, 2014

Thanks.

As they are now, the measures are completely broken but I guess nobody will notice for a long time as the basic functionality is not compromised.

Take your time ;)

@thedeerchild
Copy link
Contributor

It appears to be coming from this line, which wraps the classes, since the @reveal-modal-base mixin has media queries inside it as well.

@rafibomb rafibomb added this to the 5.3 milestone Apr 14, 2014
@rafibomb rafibomb assigned hellapixels and unassigned zurbrandon Jun 11, 2014
@rafibomb rafibomb modified the milestone: 5.3 Jun 11, 2014
@rafibomb rafibomb assigned smileyj68 and unassigned hellapixels and smileyj68 Jun 23, 2014
@rafibomb rafibomb added this to the 5.3.1 milestone Jun 23, 2014
@rafibomb rafibomb modified the milestones: 5.3.2, 5.3.1 Jul 14, 2014
@rafibomb rafibomb modified the milestones: 5.4, 5.3.2 Jul 22, 2014
@rafibomb rafibomb assigned zurbrandon and unassigned smileyj68 Jul 22, 2014
@zurbrandon
Copy link
Contributor

yeah, was this line. Added in haste at one point and has now been removed for 5.3.2

Cheers all!

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

No branches or pull requests

7 participants