Skip to content

WIP: Removes *zoom#222

Closed
peterramsing wants to merge 56 commits into7.0.0from
remove-zoom
Closed

WIP: Removes *zoom#222
peterramsing wants to merge 56 commits into7.0.0from
remove-zoom

Conversation

@peterramsing
Copy link
Copy Markdown
Owner

See #194 #135

  • Updates lost-center
  • Updates lost-masonry-wrap
  • Updates lost-utility
  • Update tests

* Updates lost-center
* Updates lost-masonry-wrap
* Updates lost-utility
@peterramsing
Copy link
Copy Markdown
Owner Author

@corysimmons @wyze Do you think that this issue regarding the test is a blocker or something to be addressed in a larger project later?

@corysimmons
Copy link
Copy Markdown
Contributor

Your call. I'd advise against merging stuff that breaks tests in any way unless you have a really thorough understanding of what is breaking the test. /$.02

@peterramsing
Copy link
Copy Markdown
Owner Author

@corysimmons Sounds good. I'm going to dive a bit deeper. This is a 7.0 thing so it can certainly wait.

@wyze
Copy link
Copy Markdown
Contributor

wyze commented Jan 7, 2016

You mentioned wanting to test the AST, so I looked into how to approach that. You would need to modify check.js and instead of using the .css property of the processor, you would want to use .root.nodes.

I would probably move the assertion out and do it in each test individually since testing the AST is more involved than a simple string check.

In case you were curious, the reason for the current testing structure as I wasn't sure how people were testing PostCSS plugins, so I checked out a few and saw what they did and mimicked it.

@peterramsing
Copy link
Copy Markdown
Owner Author

I was actually just looking at PostCSS's autoprefixer yesterday and you're right-they test the way you made it. I'll look into this further.

@peterramsing peterramsing changed the title Removes *zoom [WIP] Removes *zoom Jan 8, 2016
peterramsing and others added 13 commits January 24, 2016 20:09
* Addresses #249 by removing the grid comparison table
* Removes 6.7.0 roadmap since it has been released
* Removes the transitional note
…n) | #153

* Updates tests to reflect this change
* Updates the lost-column and lost-waffle to reflect change
* Updates documentation to reflect this change
Fixes issue where older Android wouldn't support :nth-child(n)
Since Less 2.6 you don't need to escape custom at-rules anymore.
Updates README.md to reflect updates to Less
Fixes too much code generation in waffle grids. #256
@peterramsing peterramsing changed the title [WIP] Removes *zoom WIP: Removes *zoom Jun 15, 2016
No Breaking Changes.

This simply updates the year and adds other developers to license.
For some reason I can't get NPM to download the beta changes. I'm not sure what
is causing this so I'm bumping the version and publishing a new beta to ensure
the latest changes are in. 👍
I think it's probably time I add myself as an author along with update the
homepage. 👍
Wei Wang and others added 5 commits July 5, 2016 18:13
This does not only affect tests. This also
makes a difference when building multiple
postcss targets in sequence.
After #312, these "resets" are no longer necessary.
@wbruno
Copy link
Copy Markdown
Contributor

wbruno commented Jul 6, 2016

what do we need to merge this PR ?

@peterramsing
Copy link
Copy Markdown
Owner Author

@wbruno I just need to update the tests. It hasn't been a deal-breaker so I haven't put much focus on it. I can finish it up for the next release.

Are you experiencing any issues with it or just wanting less code?

@peterramsing peterramsing added this to the v7.1.0 milestone Jul 6, 2016
@peterramsing peterramsing self-assigned this Jul 6, 2016
@wbruno
Copy link
Copy Markdown
Contributor

wbruno commented Jul 6, 2016

I have to validate my css, at w3c. CEO orders.
I can help with te tests, let me see.

@peterramsing
Copy link
Copy Markdown
Owner Author

@wbruno The gist is that I was sort of hacking the tests around the code. That was right when I took over. I'll look too before I need to duck into the office. Maybe we can get a patch out... or is this a breaking change? Crumbs...

@peterramsing
Copy link
Copy Markdown
Owner Author

Because there is no direction mention of IE7 support I don't believe this is a breaking change. Going to run this by a colleague to check my reasoning on that.

@wbruno
Copy link
Copy Markdown
Contributor

wbruno commented Jul 6, 2016

Yep! it really doesn't make any sense, keep a ie7 hack, with calc and all the others stuffs..

@peterramsing
Copy link
Copy Markdown
Owner Author

@wbruno Hehe. This is so old the target branch isn't correct. Whoops. Going to close and open a new PR.

peterramsing added a commit that referenced this pull request Jul 6, 2016
Because IE7 support is not offered for calc(), the zoom: 1 isn't needed and
simply clutters up the syntax. As seen in #222
@peterramsing peterramsing removed this from the v7.1.0 milestone Jul 6, 2016
@peterramsing peterramsing deleted the remove-zoom branch July 27, 2016 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants