search

unmade / am-date-picker

angular material date picker
MIT License
21 stars 13 forks source link

Question about the Configuration Provider #6

Closed SirDarquan closed 8 years ago

SirDarquan commented 8 years ago

In your demo, you set most of the configuration stuff in the configuration phase of bootstrap and there's nothing wrong with that. Was it you intention to only make that stuff settable in the config phase? The reason I ask is because the service portion of the provider has the same functions and can basically rewrite what was setup in the config stage.

unmade commented 8 years ago

No. It's my mistake. I don't want this behaviour. Will fix it in the next commit. When I wrote picker I needed to set some global settings, and don't really thought about service portion of provider. Thank you

SirDarquan commented 8 years ago

When I saw it doing that I fixed it immediately. But then I realized that I didn't know the intentions of the code. So with that said, I can send you a PR. Unfortunately, I've noticed a few odd things about the way the project is setup for development and I'm trying to deal with that. I'd love to talk to you about it. Should I start a new 'issue' or use this thread?

unmade commented 8 years ago

You could use that thread if you'd like. I'm still learning, so it would be great to talk about project setup for the development. PR would be great too.

SirDarquan commented 8 years ago

You know what, after really thinking about it maybe this setup isn't really odd at all. But my protractor tests no longer work. I didn't think to do all the testing before I started changing things. I've never used protractor before so was it supposed to work? And do you have any tips for checking the configuration is correct? I'll send the PR after I've gotten the tests working again

unmade commented 8 years ago

Thank you for PR. Everything seems to work fine and all tests passed. Before this project I've never used protractor too. Just wanted to see what it's like. I don't have any trouble with it, except it is a little bit long to run tests and you couldn't switch to other apps, otherwise tests fails (don't know why).

unmade commented 8 years ago

Accepted PR that fixed things.