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

Added md-readonly attribute to md-input, md-radio, md-checkbox and md-switch. #382

Merged
merged 1 commit into from
Mar 20, 2017
Merged

Conversation

HIRANO-Satoshi
Copy link
Collaborator

Hi,

I added md-readonly attribute to md-input, md-radio, md-checkbox and md-switch, since I needed readonly only input fields to show values.

If you merge this PR, please write about it in your document.

I think it would be much better if we could have a very versatile option such as md-option to give any kind of attribute.

I also fixed a typo in switch.html from <input .... > to <input .../>.

@Thanood
Copy link
Collaborator

Thanood commented Mar 11, 2017

Hi @HIRANO-Satoshi,

thanks for the contribution and feedback. 😃
I see you removed a test (or trial) with that mdOption attribute. How did that work out? It's an interesting idea.

Anyway, would you mind changing two things so I can merge you PR?

  • please remove the dist folder from the commit to have a clean setup
  • please change commit messages to adhere to out commit message conventions for easier changelog generation - f.i.: feat(md-input): add md-readonly

As for the last point: it's enough to do that with another commit when removing the dist folder, not necessary to change existing commit messages.

Thanks! 👍

@HIRANO-Satoshi
Copy link
Collaborator Author

Sorry. I will change and submit again.

The master has the dist and generated files. Should I delete them and commit?

I wanted to do the following. But I think ${mdOption} did not work. Is there a way to add the attributes?

// usage.html
<md-tag md-option="attribute1 attribute2"
// md-tag.js
@bindable() mdOption = "";
// md-tag.html
<tag ${mdOption}

@Thanood
Copy link
Collaborator

Thanood commented Mar 13, 2017

Should I delete them and commit?

I think that should do it. 😃

Regarding mdOption:
I think you would need to parse the input and use element.addAttribute(...) directly. Or split the parsed options into attributes and bind them. With the former approach you'll also be able to have both single attributes and mdOption. But that is just a guess, I didn't actually try it.

@HIRANO-Satoshi
Copy link
Collaborator Author

I did. If I need to do more, please tell me.

At this moment I'm satisfied with the readonly. I think it would be better for Aurelia to have "catch all other attributes" or something like that.

@Thanood
Copy link
Collaborator

Thanood commented Mar 20, 2017

Hi, sorry for not merging yet, I will do it today.

@Thanood Thanood merged commit 5640dac into aurelia-ui-toolkits:master Mar 20, 2017
@HIRANO-Satoshi
Copy link
Collaborator Author

Thanks.

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.

2 participants