-
Notifications
You must be signed in to change notification settings - Fork 2
Swipe menu ng build issue #4
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
@SeanKelly369 + @farfromrefug - would this commit not break every other angular plugin in the nativescript-community that makes use of this plugin seed?
For example take a look at the install.module in this plugin (which also relies on this angular seed project):
https://github.com/nativescript-community/ui-material-components
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.
@darklight365. Yes it Will be used in a repos now.
But why would it break ?
Uh oh!
There was an error while loading. Please reload this page.
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.
@farfromrefug - The module now hard code includes the ui-collectionview components and does no longer dynamically include the COMPONENTS via the plugins install.module.
Check here:
plugin-seed-demo-ng/src/app/app.module.ts
Line 18 in 8258651
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.
Good catch it is bad !
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.
Prior to this commit the seed project would import the COMPONENTS from the demo-snippets install.module, as seen here:
https://github.com/nativescript-community/ui-material-components/blob/a8a588bfb9658b3974901223c74e8a1442af20d0/demo-snippets/ng/install.module.ts#L29
With this change: the only components included in the angular test project is the ui-collectionview components.
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.
That was a try to fix build errors on angular demos.
Buy you got it to work in swipemenu without this? I mean did you revert this to have it work in swipemenu?
Uh oh!
There was an error while loading. Please reload this page.
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.
No, I did a clean checkout from master (force updated my branch). So the SwipeMenu changes worked with the current ng project seed.
I would need to perform some additional tests to see if reverting the ng seed app.module has any build issues still.
Uh oh!
There was an error while loading. Please reload this page.
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.
@farfromrefug - I've made the Angular seed generic again with this PR: #6
The other piece of the puzzle is to correctly export the SwipeMenuComponent in the demo-snippets/ng, PR for that is:
nativescript-community/ui-collectionview#80
cc: @SeanKelly369 for info