Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(docs): Build on Windows results in broken docs site and tests #4227

Closed
wants to merge 1 commit into from

Conversation

Splaktar
Copy link
Member

Multi-line SCSS comments result in ILLEGAL_CHARACTER errors in Chrome when built with Windows.
Removed unused $mdMenu injections in MenuDirective and fixed duplicate offsets variable.

Fixes #4226. Fixes #4079.

Multiline SCSS comments result in ILLEGAL_CHARACTER errors in Chrome when built with Windows.
Removed unused $mdMenu injections in MenuDirective and fixed duplicate offsets variable.
@@ -259,16 +259,16 @@ function MenuController($mdMenu, $attrs, $element, $scope) {
* the offset of top and left in pixels.
*/
function offsets() {
var offsets = ($attrs.mdOffset || '0 0').split(' ').map(parseFloat);
if (offsets.length == 2) {
var topLeftOffsets = ($attrs.mdOffset || '0 0').split(' ').map(parseFloat);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Splaktar - let's resist these types of changes in the future; as they only add noise to the diff log while offering minimal clarity improvements.

@ThomasBurleson
Copy link
Contributor

@Splaktar - I do not see duplicate offsets variable in _menu.js

Good fix on the multi-line comments. Thx.

@ThomasBurleson ThomasBurleson added this to the 0.11.0 milestone Aug 19, 2015
@ThomasBurleson ThomasBurleson self-assigned this Aug 19, 2015
@ThomasBurleson ThomasBurleson added the pr: merge ready This PR is ready for a caretaker to review label Aug 19, 2015
@Splaktar Splaktar closed this in 35ce9b8 Aug 19, 2015
@Splaktar
Copy link
Member Author

WebStorm complained due to the this.offsets at the top of the file.

@Splaktar Splaktar deleted the wip/fixDocsBuildOnWindows branch August 20, 2015 03:06
@paulflo150
Copy link
Contributor

@Splaktar - could you please take another look at this, I am getting the injector error again on win7 because _menu.js gets appended last?

Uncaught Error: [$injector:nomod] http://errors.angularjs.org/1.4.4/$injector/nomod?p0=material.components.menu(anonymous function) @ angular.js:38(anonymous function) @ angular.js:1983a @ angular.js:1907Q.bootstrap @ angular.js:1981(anonymous function) @ angular-material.min.js:11(anonymous function) @ angular-material.min.js:11(anonymous function) @ angular-material.min.js:18
angular.js:38 Uncaught Error: [$injector:modulerr] http://errors.angularjs.org/1.4.4/$injector/modulerr?p0=docsApp&p1=Error%3A…ogleapis.com%2Fajax%2Flibs%2Fangularjs%2F1.4.4%2Fangular.min.js%3A19%3A381)

Looking at the menu folder, windows explorer definitely lists _menu.js at the top, however listing the contents from a command line it's all the way at the bottom. For some reason it doesn't seem to like the underscore - if I rename it to something like menu-a.js it builds fine.

image

image

Thanks!

@Splaktar
Copy link
Member Author

@paulflo150 Yes, I can take another look. I tried to reproduce what you are seeing last week, but could not. Perhaps because I was on Windows 10 instead of Windows 7. Can you please submit a separate issue with these specifics and mention me on it?

@Splaktar
Copy link
Member Author

@paulflo150 are you all set with this now or is this still a problem for you?

@paulflo150
Copy link
Contributor

Yup, it looks great now, thanks!

kennethcachia pushed a commit to kennethcachia/material that referenced this pull request Sep 23, 2015
Multiline SCSS comments result in ILLEGAL_CHARACTER errors in Chrome when built with Windows.
Removed unused $mdMenu injections in MenuDirective and fixed duplicate offsets variable.

Fixes angular#4226. Closes angular#4227.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: Tests and site broken when built from Windows Docs failed to install at locally
3 participants