-
Notifications
You must be signed in to change notification settings - Fork 67
New Component: Inputs #164
base: main
Are you sure you want to change the base?
Conversation
Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA). 📝 Please visit http://contribute.jquery.org/CLA/ to sign. After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know. If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check. |
|
"dist/chassis", | ||
"mixins"; | ||
|
||
input[type=text], |
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.
What about input[type=date]
etc.?
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.
They're coming. I will keep updating this PR just like the other two I have opened until all is done. Like @geekman-rohit suggested to open a PR so that the code can be reviewed early on as I go.
I agree with @cvrebert - this is something we had discussed in the meeting about how styling default could break 3rd party items. |
<!-- <link href="http://fonts.googleapis.com/css?family=Open+Sans:400,600,700,400italic,700italic" rel="stylesheet"> --> | ||
</head> | ||
<body> | ||
<h1>Inputs</h1> |
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.
Might be useful to have labels and inputs together in this demo. Also some text description about the different inputs: http://view.css-chassis.com/master/demos/buttons.html
Is the |
name: "Transition animations", | ||
value: { | ||
"focus": "all .25s", | ||
"blur": "all .1s" |
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.
Should we be setting only the properties we want to transition here instead of using "all"? A quick google search gives me the impression it will be better performance wise to call out the properties.
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 call! Relevant SO http://stackoverflow.com/a/8962837/1218380
Regarding the sizes, perhaps use maps so that users can add additional sizes just by adding entries to the maps? |
<h2>Inputs - (Size: Default)</h2> | ||
|
||
<h3>Checkboxes</h3> | ||
<!-- Custom check boxes --> |
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.
Why are these inline but radio buttons aren't?
<!-- Custom Radio button --> | ||
|
||
<h3>Radio buttons</h3> | ||
|
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.
Need a fieldset with a legend for this group of radio inputs
You can view these changes at: http://view.css-chassis.com/input-styling/demos/inputs.html |
Hey your tests are failing. "There is an extra comma following the final element of an array or object literal at scss/variables/inputs.js." |
value: { | ||
"inset": "inset 0px 2px 2px 0px rgba(0,0,0,0.10)", | ||
"focus": "0px 0px 2px 1px #03a9f4", | ||
"error": "0px 0px 2px 1px #F44336", |
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.
remove comma
Normalize has been pulled in, which should fix some layout issues. Could you please rebase those changes into your develop branch. |
@sfrisk sure! |
I have rebased with the master branch. Please approve that the layout issues are fixed and I will squash and push the commits as one |
No description provided.