-
Notifications
You must be signed in to change notification settings - Fork 581
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
Upgrade npm dependencies #8506
Upgrade npm dependencies #8506
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(for anyone else's ref, here is the link to my old branch https://github.com/microsoft/pxt/commits/updateLess which was just the less changes, I think we landed at pretty much the same point though with a few extra fixes in this change)
@@ -17,6 +17,8 @@ | |||
specify theme name below | |||
*/ | |||
|
|||
@placeholder: 'default'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this overriding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fixes a build issue with less. tbh, i have no idea what it does but here's where I got it: Semantic-Org/Semantic-UI-LESS#53
Upgrading less, semantic-ui-less, karma, and karma-mocha. Once this is merged, I'll go around and do PRs on all the targets with the necessary less changes.
I think I fixed all of the problems with updating semantic-ui-less but we should definitely do some thorough UI testing on this.
Thanks to @jwunderl for reaching into his memory palace and remembering how to fix the less build.