Skip to content
This repository was archived by the owner on Oct 12, 2021. It is now read-only.

Conversation

@mgechev
Copy link
Member

@mgechev mgechev commented Jun 23, 2016

Monkey patches the parse5 tokenizer in order to allow case sensitive parsing. This way we can use case sensitive selector in order to strip content that should not belong to the App Shell.

<html>
<body>
<Div>
<Section Title="Bar"></Section>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add more variety of mixed casing to the template?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@jeffbcross
Copy link
Contributor

A couple of comments added!

By design, Angular doesn't preserve the shellRender and shellNoRender selector attributes on the compiled elements. Refresh my memory, is the assumption of this PR that at some point in the near future we would modify the directives to actually add shellRender attributes on the nodes during pre-render time?

@mgechev mgechev force-pushed the app-shell-case-sensitivity branch from 17e204e to 53709b4 Compare June 28, 2016 08:59
@mgechev
Copy link
Member Author

mgechev commented Jun 28, 2016

@jeffbcross I addressed the comments you had. As we discussed, I'll update the shellRender and shellNoRender directives, and their specs, in order to add the corresponding attributes.

Let me know if you have any other comments before being ready to merge.

@jeffbcross
Copy link
Contributor

Opened a couple of follow-up issues: #71, #70

"e2e": "protractor",
"build_publish": "rm -rf dist && tsc -p src/tsconfig.publish.es5.json && tsc -p src/tsconfig.publish.es6.json && cp src/package.json dist/package.json"
"clean": "rm -rf dist",
"build": "ng build && tsc -p src/tsconfig.publish.es5.json && tsc -p src/tsconfig.publish.es6.json && cp src/package.json dist/app/package.json && browserify dist/app/shell-parser/index.js -s shellParserFactory > dist/app/shell-parser.js && rm -rf dist/app/shell-parser && rm -rf dist/app/vendor",
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs browserify dependency installed. I can take care of this.

Copy link
Member Author

@mgechev mgechev Jun 30, 2016

Choose a reason for hiding this comment

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

Done.

@jeffbcross
Copy link
Contributor

LGTM. Once comments are addressed, plz merge.

@mgechev mgechev force-pushed the app-shell-case-sensitivity branch from 53709b4 to 56e4ba0 Compare June 30, 2016 21:04
@mgechev mgechev merged commit fb9673a into master Jun 30, 2016
@mgechev mgechev deleted the app-shell-case-sensitivity branch June 30, 2016 21:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants