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 Input ~ input example #892

Merged
merged 3 commits into from May 9, 2018

Conversation

Projects
2 participants
@nnupoor
Contributor

nnupoor commented May 5, 2018

Issue #817 example added.

@schalkneethling schalkneethling self-requested a review May 7, 2018

@schalkneethling schalkneethling self-assigned this May 7, 2018

@schalkneethling schalkneethling added this to To do in Q2 - Sprint 2 via automation May 7, 2018

@schalkneethling schalkneethling added this to the Quarter 2 ~ Sprint 2 milestone May 7, 2018

@schalkneethling

Thanks so much for your contribution @nnupoor ~ Couple of items to address here.

@font-face {
font-family: 'Fira Sans';
src: local('FiraSans-Regular'),
url('/media/fonts/FiraSans-Regular.woff2') format('woff2');

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Nit: Code Style: indent with 4 spaces

legend {
background-color: #000;
color: #ffffff;
padding: 3px 6px;

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Nit: Code Style: indent with 4 spaces

legend {
background-color: #000;
color: #ffffff;

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Nit: Code Style: Prefer short form when applicable #ffffff => #fff

padding: 0.5rem;
border-radius: 4px;
border: 1px solid #ddd;
outline: none;

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Nit: Code Style: indent with 4 spaces. Do not remove outline as it negatively impacts accessibility, unless there is alternate styling provider for the active and focus states.

input:invalid + span:after {
content: '';
color: red;
padding-left: 5px;

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Nit: Code Style: indent with 4 spaces

<label for="display-name"> Name:
<span class="warning">*(Allows only alphabets.)</span>
</label>
<input type="text" name="display-name"

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

This element is missing an id attribute with a value that matches the for attribute of the associated label element.

<span class="validity"></span>
<label for="readonly-ip">ReadOnly:</label>
<input type="text" name="readonly-ip"

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

This element is missing an id attribute with a value that matches the for attribute of the associated label element.

placeholder="I'm read only." readonly />
<label for="disabled-ip">Disabled:</label>
<input type="text"

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

This element is missing an id attribute with a value that matches the for attribute of the associated label element.

name="disabled-ip"
value="I am disabled"
disabled />
<br/>

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Code style: Do not break between form element with br, instead wrap each label/input group in a div

value="I am disabled"
disabled />
<br/>
<input type="submit" class="submit" value="Submit"/>

This comment has been minimized.

@schalkneethling

schalkneethling May 7, 2018

Collaborator

Nit: Code style: Space before closing />

font-size: .65rem;
color: #e67d09;
}

This comment has been minimized.

@nnupoor

nnupoor May 9, 2018

Contributor

@schalkneethling Removed outline: none, 0.5rem => .5rem, spacing fixes, #ffffff => #fff in CSS

<input type="text" id="display-name" name="ip-display"
pattern="[A-Za-z\s]+"
maxlength="5" minlength="2" value="Aa" required />
<span></span>

This comment has been minimized.

@nnupoor

nnupoor May 9, 2018

Contributor

Closed tags, added id for inputs for accessibility. Removed
tags.

@schalkneethling

One minor change. r+w/c

<form>
<div>
<label for="display-name"> Name:
<span class="warning">*(Allows only alphabets.)</span>

This comment has been minimized.

@schalkneethling

schalkneethling May 9, 2018

Collaborator

Nit: Please change this to read "*(Allows only letters.)"

@nnupoor

This comment has been minimized.

Contributor

nnupoor commented May 9, 2018

@schalkneethling Updated alphabets => letters

@schalkneethling

r+ Thanks @nnupoor

@schalkneethling schalkneethling merged commit 716aebe into mdn:master May 9, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json No dependency changes
Details

Q2 - Sprint 2 automation moved this from To do to Done May 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment