Skip to content

Conversation

ElliottAYoung
Copy link
Contributor

Uses the globally accessible #required? method to check and apply HTML5 presence validations based on the associated validation data we pull from the given field_item

@@ -2,6 +2,10 @@ module Plugins
module Core
class Cell < FieldCell
view_paths << "#{Cortex::Plugins::Core::Engine.root}/app/cells"

def required?
field_item.field.validations["presence"] == true
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we've used "presence" for a while but why name the validation "presence" and not "required"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because presence is what Rails / ActiveModel use to reference required validations - basically it's just to keep it consistent with how Rails does things and not change too many keywords

@arelia arelia requested review from toastercup and MKwenhua January 30, 2017 16:18
@@ -2,6 +2,10 @@ module Plugins
module Core
class Cell < FieldCell
view_paths << "#{Cortex::Plugins::Core::Engine.root}/app/cells"

def required?
field_item.field.validations["presence"] == true
Copy link
Member

Choose a reason for hiding this comment

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

We can just return field_item.field.validations["presence"] and nothing else and achieve the same effect - no need for a comparison

@@ -14,7 +14,7 @@ def value
end

def render_select
@options[:form].select 'data[user_id]', user_data_for_select, {selected: value}
@options[:form].select 'data[user_id]', user_data_for_select, {selected: value}, required: required?
end

def user_data_for_select
Copy link
Member

Choose a reason for hiding this comment

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

I assume adding required to fields like file didn't do anything? Just checking - we said we'd try (and drop it quickly), so just making sure that was looked into

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only applied it to fields that would trigger the HTML5 validation, which means that only DateTime (text input), Float / Integer (Number Field), Tag (Text Input), Text (Text Input), and User (Dropdown Select) have the ability to have on form HTML5 required validations

@ElliottAYoung ElliottAYoung merged commit 6cfe8d1 into develop Feb 1, 2017
@ElliottAYoung ElliottAYoung deleted the topic/COR-622-Error-Message-Cue branch February 1, 2017 15:53
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.

4 participants