Skip to content
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

Add support for disabled property on bs-select and bs-form-element. #67

Conversation

oswaldoacauan
Copy link

Closes #66

@simonihmig
Copy link
Contributor

Thanks for your effort! To merge this, some component tests for the bs-form-element component would be needed however, asserting the new disabled functionality (for the different control types)!

@oswaldoacauan
Copy link
Author

@simonihmig there is tests available on the dummy app, should i add unit tests for it?

@simonihmig
Copy link
Contributor

I cannot see any automated tests anywhere? To assert the new disabled functionality you can add some simple tests to the existing component integration tests in https://github.com/kaliber5/ember-bootstrap/blob/master/tests/integration/components/bs-form-element-test.js.

@oswaldoacauan
Copy link
Author

I don't see any test for other properties too. Why should disabled have tests and autofocus and placeholder not?

@offirgolan
Copy link
Contributor

Hi there,

I think that if a form-element is disabled, it should also not show a validation class.

All you would have to do is add disabled to the dependent keys and || this.get('disabled') to the conditional block. If you guys can release this soon, that would be fantastic since we have a couple projects the need this feature. Let me know if I can help with anything to speed up the process.

@simonihmig
Copy link
Contributor

@oswaldoacauan you are right about the missing tests for other properties. Must admit I was not aware of that. However trying to improve code quality over time, I would try to have all expected features to be covered by tests. I will add the missing tests for the other properties hopefully soon, can you them for disabled, please? If you need assistance, I surely can help!

@offirgolan Good point there! @oswaldoacauan Can you please add the logic to ignore validations for disabled elements?

@oswaldoacauan
Copy link
Author

Closed in favor of #71 (read comments).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants