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 new email field type #3240

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-pans-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/fields': patch
---

Add `React.useCallback` and `React.useMem` inside of Text field view
MadeByMike marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions .changeset/late-eagles-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/fields': patch
---

Updated the `Text` field view to accept a `type` prop so it can be used by other input types
5 changes: 5 additions & 0 deletions .changeset/tough-ads-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/fields': patch
---

Created a new `Email` field type
1 change: 1 addition & 0 deletions packages/fields/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export { default as CloudinaryImage } from './types/CloudinaryImage';
export { default as Color } from './types/Color';
export { default as DateTime } from './types/DateTime';
export { default as Decimal } from './types/Decimal';
export { default as Email } from './types/Email';
export { default as File } from './types/File';
export { default as Float } from './types/Float';
export { default as Integer } from './types/Integer';
Expand Down
20 changes: 20 additions & 0 deletions packages/fields/src/types/Email/Implementation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Text } from '../Text/Implementation';

export class Email extends Text {
validateInput({ resolvedData, addFieldValidationError }) {
const fieldValue = resolvedData[this.path];

if (!this.isRequired && !fieldValue) {
// If the field isn't required and we don't have a value we can skip any validation checks
return;
}

if (!this.isValidEmail(fieldValue)) {
addFieldValidationError(`Invalid email`, { value: fieldValue });
}
}

isValidEmail(email) {
return /\S+@\S+\.\S+/.test(email);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly certain it's valid for an email to not have a period: hi@io if the TLD supports it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @jesstelford! I didn't know that was a thing.

I've just updated that regex to be the same as what browsers use for input type=”email”: https://www.w3.org/TR/2012/WD-html-markup-20120329/input.email.html#input.email.attrs.value.single

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also valid: very.”(),:;<>[]”.VERY.”very@\\ "very”[email protected]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Email addresses are super weird and very hard to validate. Pretty sure you can have almost anything in the user portion since it's up to the receiving server to interpret it. I believe you also don't need a dot in the host portion as you can refer to hosts on a local network, eg. me@localhost is valid (if I'm the kind of person to run a personal mail server on my own machine where I'm also hosting Keystone I guess and, for some reason, emailing myself.. 🤷🏻‍♂️).

Glad we're delegating all this to the W3C suggested regex!

}
}
28 changes: 28 additions & 0 deletions packages/fields/src/types/Email/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!--[meta]
section: api
subSection: field-types
title: Email
[meta]-->

# Email

The `Email` field type is similar to the `Text` field, but it will validate against non valid email addresses

## Usage

```js
const { Email } = require('@keystonejs/fields');

keystone.createList('User', {
fields: {
email: { type: Email },
},
});
```

## Config

| Option | Type | Default | Description |
| ------------ | --------- | ------- | --------------------------------------------------------------- |
| `isRequired` | `Boolean` | `false` | Does this field require a value? |
| `isUnique` | `Boolean` | `false` | Adds a unique index that allows only unique values to be stored |
17 changes: 17 additions & 0 deletions packages/fields/src/types/Email/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { importView } from '@keystonejs/build-field-types';
import { MongoTextInterface, KnexTextInterface } from '../Text/Implementation';
import { Email } from './Implementation';

export default {
type: 'Email',
implementation: Email,
views: {
Controller: importView('../Text/views/Controller'),
Field: importView('./views/Field'),
Filter: importView('../Text/views/Filter'),
},
adapters: {
mongoose: MongoTextInterface,
knex: KnexTextInterface,
},
};
9 changes: 9 additions & 0 deletions packages/fields/src/types/Email/views/Field.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/** @jsx jsx */
import { jsx } from '@emotion/core';
import TextField from '../../Text/views/Field';

const EmailField = props => {
return <TextField {...props} type="email" />;
};

export default EmailField;
28 changes: 20 additions & 8 deletions packages/fields/src/types/Text/views/Field.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,33 @@
/** @jsx jsx */

import { jsx } from '@emotion/core';
import { useCallback, useMemo } from 'react';

import { FieldContainer, FieldLabel, FieldDescription, FieldInput } from '@arch-ui/fields';
import { Input } from '@arch-ui/input';

const TextField = ({ onChange, autoFocus, field, errors, value = '', isDisabled }) => {
const handleChange = event => {
onChange(event.target.value);
};
const isAccessDeniedError = error => error instanceof Error && error.name === 'AccessDeniedError';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here are mainly just moving things around / using useCallback and useMemo.


const TextField = ({
onChange,
autoFocus,
field,
errors,
value = '',
isDisabled,
type = 'text',
}) => {
const { isMultiline } = field.config;
const htmlID = `ks-input-${field.path}`;
const canRead = errors.every(
error => !(error instanceof Error && error.name === 'AccessDeniedError')
const canRead = useMemo(() => errors.every(error => !isAccessDeniedError(error)), [errors]);
const error = useMemo(() => errors.find(isAccessDeniedError), [errors]);

const handleChange = useCallback(
event => {
onChange(event.target.value);
},
[onChange]
);
const error = errors.find(error => error instanceof Error && error.name === 'AccessDeniedError');

return (
<FieldContainer>
Expand All @@ -26,7 +38,7 @@ const TextField = ({ onChange, autoFocus, field, errors, value = '', isDisabled
autoComplete="off"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a dumb call-out on my behalf; but should we not enable autoComplete for email addresses?

autoFocus={autoFocus}
required={field.isRequired}
type="text"
type={type}
value={canRead ? value : undefined}
placeholder={canRead ? undefined : error.message}
onChange={handleChange}
Expand Down