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

Text field editors #2946

Merged
merged 2 commits into from Dec 31, 2018

Conversation

Projects
None yet
3 participants
@agriffard
Copy link
Member

agriffard commented Dec 26, 2018

Related to #2492

  • Email editor
  • Color editor
  • Tel editor
  • Url editor
Text field editors
Related to #2492
@sebastienros

This comment has been minimized.

Copy link
Member

sebastienros commented Dec 31, 2018

I only see Email

@agriffard

This comment has been minimized.

Copy link
Member Author

agriffard commented Dec 31, 2018

Ok, I added the missing text field editors.

@sebastienros sebastienros merged commit 8ae921b into dev Dec 31, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details

@sebastienros sebastienros deleted the textFieldEditors branch Dec 31, 2018

psijkof pushed a commit to psijkof/OrchardCore that referenced this pull request Jan 2, 2019

@{
string currentEditor = Model.Editor;
}
<option value="Tel" selected="@String.IsNullOrEmpty(currentEditor)">@T["Phone"]</option>

This comment has been minimized.

@Skrypt

Skrypt Jan 9, 2019

Contributor

This is not right. It should be @(currentEditor == "Tel")

@{
string currentEditor = Model.Editor;
}
<option value="Url" selected="@String.IsNullOrEmpty(currentEditor)">@T["Url"]</option>

This comment has been minimized.

@Skrypt

Skrypt Jan 9, 2019

Contributor

This is not right. It should be @(currentEditor == "Url")

@{
string currentEditor = Model.Editor;
}
<option value="Color" selected="@String.IsNullOrEmpty(currentEditor)">@T["Color"]</option>

This comment has been minimized.

@Skrypt

Skrypt Jan 9, 2019

Contributor

This is not right. It should be @(currentEditor == "Color")


<fieldset class="form-group">
<label asp-for="Text">@Model.PartFieldDefinition.DisplayName()</label>
<input asp-for="Text" class="form-control content-preview-text" type="tel" />

This comment has been minimized.

@Skrypt

Skrypt Jan 9, 2019

Contributor

This should also add a pattern="[0-9]{3}-[0-9]{3}-[0-9]{4}" attribute to the input else this will render as a simple textbox. Right now this input has no validation ; I could add letters to it, though someone could say that this is for supporting phone numbers like "1-800-MY-APPLE"

This comment has been minimized.

@sebastienros

sebastienros Jan 9, 2019

Member

Don't add any validation, or more patterns. It's very locale specific, and leaving the browser do that is fine right now. Otherwise just remove the editor.

This comment has been minimized.

@Skrypt

Skrypt Jan 9, 2019

Contributor

We could give the option of a pattern and if the pattern is null then don't render one. Else this field is a textbox.

This comment has been minimized.

@sebastienros

sebastienros Jan 9, 2019

Member

Too complex. At that point better teach them how to write a custom editor. There are way too many options to handle phone numbers.

This comment has been minimized.

@Skrypt

Skrypt Jan 9, 2019

Contributor

Documentation then

agriffard added a commit that referenced this pull request Jan 9, 2019

sebastienros added a commit that referenced this pull request Jan 9, 2019

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