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

WIP: Fix media indent #26

Closed
wants to merge 2 commits into from
Closed

WIP: Fix media indent #26

wants to merge 2 commits into from

Conversation

kewah
Copy link
Contributor

@kewah kewah commented Sep 7, 2015

WIP: do not merge

I've started to fix the indent for nested @media declaration:

Current output

.foo {

  @media (max-width: 1000px) {
  display: none;
  }
}

Expected:

.foo {
  @media (max-width: 1000px) {
    display: none;
  }
}

So far I have fixed the @media indent but not the properties indent.
WIP output:

.foo {
  @media (max-width: 1000px) {
  display: none;
  }
}

I've looked at getNestedRulesNum in lib/util.js but apparently it returns parent.type === 'root' for display: none;.

Am I looking at the right place? Do you have any clue on how to fix this indent issue?
Do you think it's a postcss issue?

Thanks.

@matype
Copy link
Owner

matype commented Sep 7, 2015

Wow! Thank you always :)
If you have probrems in my code, please feel free to ask me.

@kewah
Copy link
Contributor Author

kewah commented Sep 7, 2015

@morishitter I didn't find a way to fix the indent issue inside the media declaration. If you have any suggestions/clues? :)

I've looked at getNestedRulesNum in lib/util.js but apparently it returns parent.type === 'root' for display: none;.

@matype
Copy link
Owner

matype commented Sep 7, 2015

OK, I will confirm it.

@matype matype mentioned this pull request Sep 15, 2015
@matype
Copy link
Owner

matype commented Sep 15, 2015

@kewah I just fixed this, and released v1.3.3. e46f1eb 35532d3

Please update cssfmt to latest version. Thank you for your report :)

@matype matype closed this Sep 15, 2015
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.

2 participants